Invalid cql-json and expression is parsed into "wrong" ast

[AsgerPetersen]

I cannot find any description of how pygeofilter is supposed to behave when it meets invalid filters. The issue is written assuming that pygeofilter should fail when parsing invalid filters.

Example 1 where pygeofilter silently ignores invalid and expression:

>>> import pygeofilter.ast
>>> from pygeofilter.parsers.cql_json import parse as parse_json
>>> 
>>> cql_json = { "intersects": [ { "property": "geometry" }, { "type": "Point", "coordinates": [ 10.4064, 55.3951 ] } ], "and": [ {"eq": [ { "property": "direction" }, "east" ] } ] }
>>> print(pygeofilter.ast.get_repr(parse_json(cql_json)))
INTERSECTS(ATTRIBUTE geometry, Geometry(geometry={'type': 'Point', 'coordinates': [10.4064, 55.3951]}))

Example 2 where pygeofilter parses invalid and into a simple comparison:

>>> import pygeofilter.ast
>>> from pygeofilter.parsers.cql_json import parse as parse_json
>>> 
>>> cql_json = { "and": [ {"eq": [ { "property": "direction" }, "east" ] } ] }
>>> print(pygeofilter.ast.get_repr(parse_json(cql_json)))
ATTRIBUTE direction = 'east'

Needless to say it would be a better user experience if both these examples would fail with some meaningful message.

[constantinius]

@AsgerPetersen

Thanks for reporting this issue. The JSON based parsers are indeed rather simple and will ignore extra attributes of the identified node. But I agree, extra arguments should not be silently ignored but reported as errors. Contributions are welcome.

Regarding the and with only a single item: I concur, we should add a check to mark these cases as errors!

[bitner]

I disagree about the and with a single item -- in the case where someone is constructing a query where they want all filters to be considered (ie joined with and), allowing "and" with an array with a single item allows the query constructor to not need to check if they have an array and to then further decide whether to wrap with the "and" or not. Think of it like split in python -- ','.split('test') still gives a single element array.

…

On Wed, Jun 8, 2022 at 8:07 AM Fabian Schindler ***@***.***> wrote: @AsgerPetersen <https://github.com/AsgerPetersen> Thanks for reporting this issue. The JSON based parsers are indeed rather simple and will ignore extra attributes of the identified node. But I agree, extra arguments should not be silently ignored but reported as errors. Contributions are welcome. Regarding the and with only a single item: I concur, we should add a check to mark these cases as errors! — Reply to this email directly, view it on GitHub <#47 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABIHXG4KWLOL4YPQW3TPITVOCLJJANCNFSM5YGBXMIQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

-- ************************************ David William Bitner dbSpatial LLC 612-578-9553

[AsgerPetersen]

Thank you both. Unfortunately I there isn´t enough time for me to participate actively in all the great projects I use. I try to my best to take the time to at least write decent bug reports.

@bitner that makes sense, but doesn´t the spec require at least 2 items? So other parsers would probably behave differently.

[constantinius]

Unfortunately I there isn´t enough time for me to participate actively in all the great projects I use. I try to my best to take the time to at least write decent bug reports.

Your contribution in the form of bug reports is very much appreciated! I did not mean to impose further work on you, but rather something I don't have the resources to fix right now. So if anyone wants to step up and take this over, I'd be very grateful!

@bitner that makes sense, but doesn´t the spec require at least 2 items? So other parsers would probably behave differently.

That is indeed true, the spec mandates at least 2 items

[bitner]

On Wed, Jun 8, 2022, 5:42 AM Asger Skovbo Petersen ***@***.***> wrote: I cannot find any description of how pygeofilter is supposed to behave when it meets invalid filters. The issue is written assuming that pygeofilter should fail when parsing invalid filters. Example 1 where pygeofilter silently ignores invalid and expression: >>> import pygeofilter.ast >>> from pygeofilter.parsers.cql_json import parse as parse_json >>> >>> cql_json = { "intersects": [ { "property": "geometry" }, { "type": "Point", "coordinates": [ 10.4064, 55.3951 ] } ], "and": [ {"eq": [ { "property": "direction" }, "east" ] } ] } >>> print(pygeofilter.ast.get_repr(parse_json(cql_json))) INTERSECTS(ATTRIBUTE geometry, Geometry(geometry={'type': 'Point', 'coordinates': [10.4064, 55.3951]}))

This example does look like the wrong behavior. Example 2 where pygeofilter parses invalid and into a simple comparison:

>>> import pygeofilter.ast >>> from pygeofilter.parsers.cql_json import parse as parse_json >>> >>> cql_json = { "and": [ {"eq": [ { "property": "direction" }, "east" ] } ] } >>> print(pygeofilter.ast.get_repr(parse_json(cql_json))) ATTRIBUTE direction = 'east'

In this case, the and is perfectly valid (just useless and so it semantically drops out).

…

Needless to say it would be a better user experience if both these examples would fail with some meaningful message. — Reply to this email directly, view it on GitHub <#47>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABIHXHDN4PH5ABBKOVRMTLVOB2HVANCNFSM5YGBXMIQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>