-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
filter-lang parameter not being passed correctly to pgstac backend causing incorrect response for listing items in a collection #29
Comments
Actually, as a quick update: I've found an alteration to the code that seems to work. Adding seems to fix it. It seems like the I could be completely wrong here, as I'm not experienced with pydantic or FastAPI. If I'm right then I'd be more than happy to do a PR. |
It seems I faced the similar issue here. Expected behaviorSearch using Current behaviorSearch with
It just returns everything. WorkaroundEnabling FilterExtension makes Packages
cc @bitner |
Thanks @drnextgis - I think you're right in this issue and in stac-utils/stac-fastapi#333, it seems like the FilterExtension needs to be enabled to allow various bits of base functionality (like viewing a specific item by ID, or viewing items in a collection). It turned out that I hadn't enabled the FilterExtension in this particular clone of the repo, and by enabling it I don't need to add my line of code to define the filter_lang. This raises the question of whether the FilterExtension needs to be enabled by default, as it seems some of the base functionality depends on it? |
My opinion is that FilterExtension should not be enabled by default right now, as it is not currently compliant with the latest (1.0.0-beta.5) version of FilterExtension. This would cause all users to be affected by the future change in behavior to become compliant, instead of users opting-in to that. |
That's a good point, but I'm concerned that it seems that basic functionality like listing the items in a collection (using the URL |
Right, I just think it needs to be fixed somehow without enabling the filter extension. |
can we try to write a test for this? Maybe @bitner can 👀 this too. If pgstac database requires |
Right now, pgstac using cql2-json does not support use of the ids, collections, datetime, bbox, intersects, or query top level (https://github.com/stac-utils/pgstac/blob/main/CHANGELOG.md#v040). As of now, if you are using cql2-json, everything must be included in the filter. The only stac extension that can be directly enabled/disabled in pgstac is the context extension, otherwise in pgstac filter is always enabled. The (deprecated) query extension is essentially disabled if the filter-lang is set to cql2-json. There may be a bug in the stac-fastapi-pgstac implementation for collections// and // when used with cql2-json as those endpoints are just passing on the collections= and ids= parameters to the backend which is then ignoring them. I'll work to get a fix in to enable collections, ids, datetime, bbox, and intersects parameters, but I am not planning to add usage of the (deprecated) query parameter along with cql2-json. Help and PR's are always welcome here or in pgstac! Particularly if there is any documentation or tests that can be added to match a broader set of use cases. Otherwise, I tend to have blinders on towards the way that the implementation of stac-fastapi and pgstac that I am working on has. If anyone would like a walk through of anything to be able to help out, let me know! |
@philvarner As far as my latest reading of the spec, the PGStac cql2-json implementation should be compliant with the latest version (1.0.0-beta.5) version of FilterExtension. It has tests for all the examples that are currently on the spec. Are there any holes or bugs that you are aware of (if so, it would be great if you could add an issue on the pgstac repo). |
@bitner great -- I wasn't expecting the recent changes around the structure of the json to have been made yet, so it's great that they have been. |
Support for ids, collections, datetime, intersects, and bbox while using filter-lang=cql2-json has been added to pgstac in the latest pgstac PR (to become pgstac v0.4.4). Once this is merged in pgstac, we can remove some of the auto-version detection which I don't think is working how we want and bump the version in stac-fastapi. This does NOT, however add support for using the deprecated query parameter along with filter-lang=cql2-json. |
@robintw @drnextgis Can you confirm whether this issue has been fixed in the most recent version of |
I've recently upgraded to the latest version of stac-fastapi, and upgraded my pgstac database too.
I've found problems where going to a URL like
/collections/collection-name
gives results that include items from all collections, rather than filtering results to just items from a single collection.I raised an issue on the pgstac repository, as I thought it was a pgstac problem. They told me that it was due to changes in v4 of pgstac, which require the
filter-lang
parameter to be passed, or alternatively to be set in the pgstac settings to be able to use cql-json rather than cql2-json. I'm continuing to talk to them on that issue as I can't seem to get my change to pgstac settings to be recognised, but I've tested using raw SQL queries and I do get the correct results when I pass thefilter-lang
parameter.However, back in stac-fastapi: in stac-utils/stac-fastapi#308, the upgrade was made to use pgstac 4.0.0, and code was added to set
filter-lang
when certain parameters (likequery
orcollections
) were passed as part of the code (see here). However, this filter-lang parameter doesn't actually seem to be getting set.I've modified the code slightly to add a print statement below this line, and that shows that when visiting a URL like
/collections/collection-name
, you get JSON of{"collections": ["collection-name"], "limit": 10, "fields": {"include": [], "exclude": []}}
- that is, without thefilter-lang
parameter.Doing some very hacky string manipulation to set the
filter-lang
parameter in that JSON fixes the results.I've been trying to debug this myself, but I don't know much about pydantic and I can't work out how
validate_query_uses_cql
is being used - it looks like the decorator means it should be called 'automatically' and set the value, but it doesn't seem to be.I'm wondering whether this problem is also the cause of stac-utils/stac-fastapi#333.
Any help would be much appreciated.
The text was updated successfully, but these errors were encountered: