Re: [Geotools-devel] Filter-CQL issues

[Devon Tucker]

I think, unfortunately, fixing this may be trickier than I had hoped. Part
of the problem is on the parsing end, because two valid property names get
parsed into the exact same thing.

"foo/bar" -> foo/bar (no quotes)
foo.bar   -> foo/bar
foo/bar   -> doesn't parse

Only the final one is correct. This is further complicated by the semantics
involved. foo.bar is parsed as a path whose parts are foo and bar, which is
then concatenated with a forward slash. "foo/bar" is initially correctly
parsed as a quoted property name, but then the quotes are stripped and then
becomes indistinguishable from foo.bar.

Also, I can't find anything in the OGC CQL spec that allows for quoted
identifiers.

OGC 07-006r1
 ::=
 |  } ]
 ::= 
 ::=  | 
 ::=  | 

 ::= 
 ::= 
  [{}...]
  

So yeah, not even really sure where to start with fixing this or even if it
is worth it for my use cases at least. For a start it should probably not
be losing semantically meaningful information on the parsing end (path vs.
quoted identifier with a forward slash in it), but right now this gets
parsed into an object model that is not quite a 1-to-1 match (OGC Filter
spec).

On Tue, Feb 5, 2019 at 10:20 AM Jody Garnett  wrote:

> In todays meeting we compared the library science CQL which uses
> "foo/bar.baz" with the OGC CSW CQL which uses "foo.bar.baz" - sigh :)
>
> - https://www.loc.gov/standards/sru/cql/spec.html
> - http://portal.opengeospatial.org/files/?artifact_id=20555
>
> --
> Jody Garnett
>
>
> On Tue, 5 Feb 2019 at 10:12, Andrea Aime 
> wrote:
>
>> HI Devon,
>> agree with the others that the best is to do a round trip test.
>> Just mind, while in memory a nested attribute reference should be xpath
>> like, a/b, while encoded
>> in CQL it should be a.b. So make sure it works fine both ways, I'd do two
>> round trip tests:
>>
>>- Start with "a/b" as PropertyName, encode and decode, you should be
>>getting back a PropertyName as "a/b"
>>- Start with "a.b" as a CQL string, decode and encode, you should get
>>back "a.b"
>>
>> Cheers
>> Andrea
>>
>> On Mon, Feb 4, 2019 at 6:41 PM Devon Tucker 
>> wrote:
>>
>>> Hi all,
>>>
>>> Working with the Filter -> CQL code recently (CQL.toCQL) I noticed that
>>> it is not producing correct CQL for attribute names. For example the
>>> following test case fails:
>>>
>>> https://gist.github.com/dvntucker/1d7f8c227eea44d9bd1563f0269e4b6a
>>>
>>> There are a number of issues here, but the biggest one is the
>>> forward-slash instead of a period, which is not correct by the CQL spec.
>>> Also, this identifier does not need to be quoted and I don't think quoting
>>> identifiers is in the CQL spec either.
>>>
>>> It looks like this is mainly an artifact from the parsing phase, where
>>> the periods are replaced with slashes to match XPath. I believe this part
>>> is correct, but needs to be corrected when producing CQL output. My only
>>> worry with changing this is potentially people relying on this behavior.
>>> Looking through GS/GT, it doesn't appear that this code path (Filter->CQL)
>>> is used a whole lot, and in fact where it is seems very likely to fail
>>> because of this (forward slash in identifier doesn't parse).
>>>
>>> Anyone see an issue with fixing this behavior?
>>>
>>> Cheers,
>>> Devon
>>> ___
>>> GeoTools-Devel mailing list
>>> GeoTools-Devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>
>>
>>
>> --
>>
>> Regards, Andrea Aime == GeoServer Professional Services from the experts!
>> Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime
>> @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054
>> Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339
>> 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it
>> --- *Con riferimento
>> alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 -
>> Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni
>> circostanza inerente alla presente email (il suo contenuto, gli eventuali
>> allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i
>> destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per
>> errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le
>> sarei comunque grato se potesse darmene notizia. This email is intended
>> only for the person or entity to which it is addressed and may contain
>> information that is privileged, confidential or otherwise protected from
>> disclosure. We remind that - as provided by European Regulation 2016/679
>> “GDPR” - copying, dissemination or use of this e-mail or the information
>> herein by anyone other than the intended recipient is prohibited. If you
>> have received this email by mistake, please notify us immediately by
>> telephone or e-mail.*
>> 

Re: [Geotools-devel] Filter-CQL issues

[Jody Garnett]

In todays meeting we compared the library science CQL which uses
"foo/bar.baz" with the OGC CSW CQL which uses "foo.bar.baz" - sigh :)

- https://www.loc.gov/standards/sru/cql/spec.html
- http://portal.opengeospatial.org/files/?artifact_id=20555

--
Jody Garnett


On Tue, 5 Feb 2019 at 10:12, Andrea Aime 
wrote:

> HI Devon,
> agree with the others that the best is to do a round trip test.
> Just mind, while in memory a nested attribute reference should be xpath
> like, a/b, while encoded
> in CQL it should be a.b. So make sure it works fine both ways, I'd do two
> round trip tests:
>
>- Start with "a/b" as PropertyName, encode and decode, you should be
>getting back a PropertyName as "a/b"
>- Start with "a.b" as a CQL string, decode and encode, you should get
>back "a.b"
>
> Cheers
> Andrea
>
> On Mon, Feb 4, 2019 at 6:41 PM Devon Tucker 
> wrote:
>
>> Hi all,
>>
>> Working with the Filter -> CQL code recently (CQL.toCQL) I noticed that
>> it is not producing correct CQL for attribute names. For example the
>> following test case fails:
>>
>> https://gist.github.com/dvntucker/1d7f8c227eea44d9bd1563f0269e4b6a
>>
>> There are a number of issues here, but the biggest one is the
>> forward-slash instead of a period, which is not correct by the CQL spec.
>> Also, this identifier does not need to be quoted and I don't think quoting
>> identifiers is in the CQL spec either.
>>
>> It looks like this is mainly an artifact from the parsing phase, where
>> the periods are replaced with slashes to match XPath. I believe this part
>> is correct, but needs to be corrected when producing CQL output. My only
>> worry with changing this is potentially people relying on this behavior.
>> Looking through GS/GT, it doesn't appear that this code path (Filter->CQL)
>> is used a whole lot, and in fact where it is seems very likely to fail
>> because of this (forward slash in identifier doesn't parse).
>>
>> Anyone see an issue with fixing this behavior?
>>
>> Cheers,
>> Devon
>> ___
>> GeoTools-Devel mailing list
>> GeoTools-Devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>
>
>
> --
>
> Regards, Andrea Aime == GeoServer Professional Services from the experts!
> Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime
> @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054
> Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339
> 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it
> --- *Con riferimento
> alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 -
> Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni
> circostanza inerente alla presente email (il suo contenuto, gli eventuali
> allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i
> destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per
> errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le
> sarei comunque grato se potesse darmene notizia. This email is intended
> only for the person or entity to which it is addressed and may contain
> information that is privileged, confidential or otherwise protected from
> disclosure. We remind that - as provided by European Regulation 2016/679
> “GDPR” - copying, dissemination or use of this e-mail or the information
> herein by anyone other than the intended recipient is prohibited. If you
> have received this email by mistake, please notify us immediately by
> telephone or e-mail.*
> ___
> GeoTools-Devel mailing list
> GeoTools-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Re: [Geotools-devel] Filter-CQL issues

[Andrea Aime]

HI Devon,
agree with the others that the best is to do a round trip test.
Just mind, while in memory a nested attribute reference should be xpath
like, a/b, while encoded
in CQL it should be a.b. So make sure it works fine both ways, I'd do two
round trip tests:

   - Start with "a/b" as PropertyName, encode and decode, you should be
   getting back a PropertyName as "a/b"
   - Start with "a.b" as a CQL string, decode and encode, you should get
   back "a.b"

Cheers
Andrea

On Mon, Feb 4, 2019 at 6:41 PM Devon Tucker 
wrote:

> Hi all,
>
> Working with the Filter -> CQL code recently (CQL.toCQL) I noticed that it
> is not producing correct CQL for attribute names. For example the following
> test case fails:
>
> https://gist.github.com/dvntucker/1d7f8c227eea44d9bd1563f0269e4b6a
>
> There are a number of issues here, but the biggest one is the
> forward-slash instead of a period, which is not correct by the CQL spec.
> Also, this identifier does not need to be quoted and I don't think quoting
> identifiers is in the CQL spec either.
>
> It looks like this is mainly an artifact from the parsing phase, where the
> periods are replaced with slashes to match XPath. I believe this part is
> correct, but needs to be corrected when producing CQL output. My only worry
> with changing this is potentially people relying on this behavior. Looking
> through GS/GT, it doesn't appear that this code path (Filter->CQL) is used
> a whole lot, and in fact where it is seems very likely to fail because of
> this (forward slash in identifier doesn't parse).
>
> Anyone see an issue with fixing this behavior?
>
> Cheers,
> Devon
> ___
> GeoTools-Devel mailing list
> GeoTools-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>


-- 

Regards, Andrea Aime == GeoServer Professional Services from the experts!
Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime
@geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054
Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339
8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it
--- *Con riferimento
alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 -
Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni
circostanza inerente alla presente email (il suo contenuto, gli eventuali
allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i
destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per
errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le
sarei comunque grato se potesse darmene notizia. This email is intended
only for the person or entity to which it is addressed and may contain
information that is privileged, confidential or otherwise protected from
disclosure. We remind that - as provided by European Regulation 2016/679
“GDPR” - copying, dissemination or use of this e-mail or the information
herein by anyone other than the intended recipient is prohibited. If you
have received this email by mistake, please notify us immediately by
telephone or e-mail.*
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Re: [Geotools-devel] Filter-CQL issues

[Emilio Lahr-Vivaz]

Yeah, I think the main concern in GeoMesa is not the CQL formatting 
per-say, but that for any given Filter, the following test holds:


Assert.assertEquals(filter, ECQL.toFilter(ECQL.toCQL(filter

(or at least that they are equivalent, if not strictly equals)

Thanks,

Emilio

On 2/4/19 4:49 PM, Jim Hughes wrote:

Hi Jody, Devon,

Thanks for asking.  From chatting with Emilio, we don't think it'd 
affect anything in GeoMesa.


From a quick look at the ECQL reference, it looks like attributes with 
non-alphanumeric characters should be double-quoted: 
https://docs.geoserver.org/latest/en/user/filter/ecql_reference.html#attribute 
?


Does that help any?

Devon, what does your unit test do? If it is calling CQL.toFilter and 
then CQL.toCQL and checking equality, that seems like an excellent 
unit test!


Cheers,

Jim


On 2/4/19 2:17 PM, Jody Garnett wrote:
I would check with the GeoMesa project as they heavily rely on toCQL 
to package up query strings. As long as James (CCed) knows when 
making to upgrade fixing the issue should be fine.


James are you aware of this issue? Or has it not come up in your 
testing...


On Mon, Feb 4, 2019 at 9:41 AM Devon Tucker > wrote:


Hi all,

Working with the Filter -> CQL code recently (CQL.toCQL) I
noticed that it is not producing correct CQL for attribute names.
For example the following test case fails:

https://gist.github.com/dvntucker/1d7f8c227eea44d9bd1563f0269e4b6a

There are a number of issues here, but the biggest one is the
forward-slash instead of a period, which is not correct by the
CQL spec. Also, this identifier does not need to be quoted and I
don't think quoting identifiers is in the CQL spec either.

It looks like this is mainly an artifact from the parsing phase,
where the periods are replaced with slashes to match XPath. I
believe this part is correct, but needs to be corrected when
producing CQL output. My only worry with changing this is
potentially people relying on this behavior. Looking through
GS/GT, it doesn't appear that this code path (Filter->CQL) is
used a whole lot, and in fact where it is seems very likely to
fail because of this (forward slash in identifier doesn't parse).

Anyone see an issue with fixing this behavior?

Cheers,
Devon
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net

https://lists.sourceforge.net/lists/listinfo/geotools-devel

--
--
Jody Garnett


___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel





___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Re: [Geotools-devel] Filter-CQL issues

[Jim Hughes]

Hi Jody, Devon,

Thanks for asking.  From chatting with Emilio, we don't think it'd 
affect anything in GeoMesa.


From a quick look at the ECQL reference, it looks like attributes with 
non-alphanumeric characters should be double-quoted: 
https://docs.geoserver.org/latest/en/user/filter/ecql_reference.html#attribute 
?


Does that help any?

Devon, what does your unit test do?  If it is calling CQL.toFilter and 
then CQL.toCQL and checking equality, that seems like an excellent unit 
test!


Cheers,

Jim


On 2/4/19 2:17 PM, Jody Garnett wrote:
I would check with the GeoMesa project as they heavily rely on toCQL 
to package up query strings. As long as James (CCed) knows when making 
to upgrade fixing the issue should be fine.


James are you aware of this issue? Or has it not come up in your 
testing...


On Mon, Feb 4, 2019 at 9:41 AM Devon Tucker > wrote:


Hi all,

Working with the Filter -> CQL code recently (CQL.toCQL) I noticed
that it is not producing correct CQL for attribute names. For
example the following test case fails:

https://gist.github.com/dvntucker/1d7f8c227eea44d9bd1563f0269e4b6a

There are a number of issues here, but the biggest one is the
forward-slash instead of a period, which is not correct by the CQL
spec. Also, this identifier does not need to be quoted and I don't
think quoting identifiers is in the CQL spec either.

It looks like this is mainly an artifact from the parsing phase,
where the periods are replaced with slashes to match XPath. I
believe this part is correct, but needs to be corrected when
producing CQL output. My only worry with changing this is
potentially people relying on this behavior. Looking through
GS/GT, it doesn't appear that this code path (Filter->CQL) is used
a whole lot, and in fact where it is seems very likely to fail
because of this (forward slash in identifier doesn't parse).

Anyone see an issue with fixing this behavior?

Cheers,
Devon
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net

https://lists.sourceforge.net/lists/listinfo/geotools-devel

--
--
Jody Garnett


___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel



___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Re: [Geotools-devel] Filter-CQL issues

[Jody Garnett]

I would check with the GeoMesa project as they heavily rely on toCQL to
package up query strings. As long as James (CCed) knows when making to
upgrade fixing the issue should be fine.

James are you aware of this issue? Or has it not come up in your testing...

On Mon, Feb 4, 2019 at 9:41 AM Devon Tucker 
wrote:

> Hi all,
>
> Working with the Filter -> CQL code recently (CQL.toCQL) I noticed that it
> is not producing correct CQL for attribute names. For example the following
> test case fails:
>
> https://gist.github.com/dvntucker/1d7f8c227eea44d9bd1563f0269e4b6a
>
> There are a number of issues here, but the biggest one is the
> forward-slash instead of a period, which is not correct by the CQL spec.
> Also, this identifier does not need to be quoted and I don't think quoting
> identifiers is in the CQL spec either.
>
> It looks like this is mainly an artifact from the parsing phase, where the
> periods are replaced with slashes to match XPath. I believe this part is
> correct, but needs to be corrected when producing CQL output. My only worry
> with changing this is potentially people relying on this behavior. Looking
> through GS/GT, it doesn't appear that this code path (Filter->CQL) is used
> a whole lot, and in fact where it is seems very likely to fail because of
> this (forward slash in identifier doesn't parse).
>
> Anyone see an issue with fixing this behavior?
>
> Cheers,
> Devon
> ___
> GeoTools-Devel mailing list
> GeoTools-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>
-- 
--
Jody Garnett
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel