Skip to content
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

Normalize double quoting across all operators #1943

Open
steve-chavez opened this issue Sep 10, 2021 · 5 comments
Open

Normalize double quoting across all operators #1943

steve-chavez opened this issue Sep 10, 2021 · 5 comments
Labels
hygiene cleanup or refactoring idea Needs of discussion to become an enhancement, not ready for implementation

Comments

@steve-chavez
Copy link
Member

steve-chavez commented Sep 10, 2021

Currently, we allow double quoting only on the in/and/or operators, this is inconsistent and is an issue for client-side libraries.

Allowing double quotes on other operators would mean a breaking change though.

For example, right now the values for eq are free-form(eq."val" would get SQL translated to = '"val"'), if we allowed double quoting on eq the quotes would get lost(eq."val" would get SQL translated to = 'val').


This was previously discussed on #1591 (comment).

At http://postgrest.org/en/latest/api.html#unicode-support it reads:
If filters include PostgREST reserved characters(,, ., :, ()) you’ll have to surround them in percent encoded double quotes %22 for correct processing.

The way it is written, I would expect this to always be the case. But this is in fact only supported for the in. operator (and maybe arrays, not sure) and inside logical operators.

This will not work, even though it should according to documentation:

GET /authors?name=eq.%22Anne%20Frank%22

But this works:

GET /authors?and=(name.eq.%22Anne%20Frank%22)

For a client-side library this is a pain to quote correctly, because context is needed. I'd consider the docs to be right and the behaviour to be a bug.


Some problems with quoting:

@steve-chavez steve-chavez added hygiene cleanup or refactoring idea Needs of discussion to become an enhancement, not ready for implementation breaking change A bug fix or enhancement that would cause a breaking change labels Sep 10, 2021
@steve-chavez
Copy link
Member Author

In hindsight, maybe we should have resorted to escaping instead of double quoting. For example, on the in operator the main problem are the commas(,), pg already allows escaping the comma value with a backslash:

select '{a,b,c}'::text[];
┌─────────┐
│  text   │
├─────────┤
│ {a,b,c} │
└─────────┘

select '{a\,b,c}'::text[];
┌───────────┐
│   text    │
├───────────┤
│ {"a,b",c} │
└───────────┘

@steve-chavez
Copy link
Member Author

steve-chavez commented Dec 2, 2021

For the problem with in above, perhaps we can add an any operator that lets the user specify a pg array, the input would be the same as our cs/cd operators. With that operator, we can leave the escaping to pg backslash handling.

Adding a new operator would avoid a breaking change on in.

@wolfgangwalther
Copy link
Member

For the problem with in above, perhaps we can add an any operator that lets the user specify a pg array, the input would be the same as our cs/cd operators.

I think we discussed elsewhere whether to maybe add an any modifier to make all operators usable with it? So what we'd be using here was col=any.eq.array, right?

@steve-chavez
Copy link
Member Author

steve-chavez commented Dec 17, 2021

So what we'd be using here was col=any.eq.array, right?

Yes, correct.

Also, to cross link this issue, on #1970 (comment) we discussed a new or that would not need quoting.

@steve-chavez
Copy link
Member Author

With the above improvements to in and or, then we would only need double quoting in case column names contain reserved characters.

GET /vulnerabilities?"information.cpe"=like.*MS
Here information.cpe is a column name.

Or we could enable backslash escaping for columns as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hygiene cleanup or refactoring idea Needs of discussion to become an enhancement, not ready for implementation
Development

No branches or pull requests

2 participants