Skip to content

Make sure that only arguments that are properties are cheked #219

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

Merged
merged 3 commits into from
Oct 5, 2023

Conversation

drnextgis
Copy link
Collaborator

Fixes #218

@drnextgis
Copy link
Collaborator Author

drnextgis commented Oct 4, 2023

I moved the code to a different location and added a loop to iterate through all properties, ensuring all of them are included in the queryables. Previously, the code was placed incorrectly in the loop which was only meant to detect the wrapper. Furthermore, I made it explicit in the code that the properties id, datetime, end_datetime, and collection are always considered queryable.

bitner
bitner previously approved these changes Oct 5, 2023
Copy link
Collaborator

@bitner bitner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Catch!

@bitner bitner dismissed their stale review October 5, 2023 16:35

Hmmm. Actually, I think that we can do this even higher up in the stack in a way that can fail on the first call of cql2_query and not need to wait until we go down the recursive tree of the query. I'll suggest something in just a minute...

@bitner
Copy link
Collaborator

bitner commented Oct 5, 2023

@drnextgis I think if we put something like this:

    IF NOT extra_props THEN
        FOR prop IN 
            SELECT DISTINCT p->>0 
            FROM jsonb_path_query(j, 'STRICT $..property')
            WHERE p->>0 NOT IN ('id', 'datetime', 'end_datetime', 'collection')
        LOOP
            IF (queryable(prop)).nulled_wrapper IS NULL THEN
                RAISE EXCEPTION 'Term % is not found in queryables.', arg->>'property';
            END IF;
        END LOOP;
    END IF;

at the very top of the cql2_query (probably right after the RAISE NOTICE 'CQL2_QUERY: %', j; it should find any property nested anywhere in the filter on the first call to cql2_query rather than when things have gone down the nesting iterations. I think this will be more resistant to any of the different implementations for spatial, array, and temporal operators.

@drnextgis
Copy link
Collaborator Author

@bitner thank you! I've updated the PR.

Copy link
Collaborator

@bitner bitner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@bitner bitner merged commit 1ea6c5d into stac-utils:main Oct 5, 2023
@drnextgis drnextgis deleted the issue-163-2 branch October 5, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception with a legitimate SQL query
2 participants