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

Allow 'in.' filter to have no items #641

Closed
daurnimator opened this issue Jun 15, 2016 · 20 comments
Closed

Allow 'in.' filter to have no items #641

daurnimator opened this issue Jun 15, 2016 · 20 comments
Labels

Comments

@daurnimator
Copy link
Contributor

When in. is not followed by any characters, treat it as the empty set.


Related to marmelab/admin-config#71

I'm using the code:

                if (params._filters[filter] instanceof Array) {
                    params[filter] = "in." + params._filters[filter].join(",");
                } else {
                    params[filter] = "eq." + params._filters[filter];
                }

However, if the filter list is empty, it ends up requesting the path /organisation?id=in.. This comes back from postgrest as a 400 error:

{"hint":null,"details":null,"code":"22P02","message":"invalid input syntax for uuid: \"\""}
@ruslantalpa
Copy link
Contributor

can you show the sql query you are trying to generate and how it is useful?

@daurnimator
Copy link
Contributor Author

can you show the sql query you are trying to generate

Doesn't matter. the result will always be an empty result set.

and how it is useful?

See the linked admin-config issue: it expects there to be an API call that returns things in set X. But then it passes an empty set as X.

@ruslantalpa
Copy link
Contributor

At this point this seems to me like a way to fix deficiencies in the client by patching the backend.
Unless there is a valid usecase for in., i would treat it as a bad request.

if postgrest treats this as a bad request

select * from test.projects p
where p.id in ()

i don't see why we should accept it as a valid request

@daurnimator
Copy link
Contributor Author

daurnimator commented Jun 15, 2016

if postgrest treats this as a bad request

select * from test.projects p
where p.id in ()

i don't see why we should accept it as a valid request

That's a reasonable argument.

The thing is, at the moment postgrest converts it to the empty string:

select * from test.projects p
where p.id in ("")

@SebAlbert
Copy link

Just in case you might be counting votes/opinions:
The empty set is a very valid set, so I strongly think that the request ought to be valid as well. (I would have expected Haskell programmers among the people most likely to agree - although that's an obvious stereotype)

@SebAlbert
Copy link

On a different note, @daurnimator and others, how would you suggest the user should put the query string if they actually want exactly this query with the empty string? In fact, you could argue that what the user put behind the "in." part is the empty string. You merely cannot distinguish the empty string from nothing, or is there a meaningful way?

@ruslantalpa
Copy link
Contributor

@SebAlbert empty set might be valid in some domains but not here, we are talking about sql and in () is an invalid sql statement

@ruslantalpa
Copy link
Contributor

i.e. please show me a query with in operator and an empty set that at least does not raise an exception (not even asking for a valid usecase)

@SebAlbert
Copy link

You're right, it's not valid. To my mind, a use case, however, would be "not having to fiddle with special cases" in frontend code, just like in this issue here.
Might it be possible to generally switch to the array "subset" operators in ng-admin / admin-config? There's no problem with checking for membership/existence in empty arrays, and hopefully not in PostgREST either. (I.e., using "<@." instead of "in.")

@begriffs
Copy link
Member

begriffs commented Jun 16, 2016

There are two reasons that I think we should allow in. to return no rows rather than raising an error.

  1. Although we know a priori that x ∈ ∅ is false, we also know for instance that ?foo=eq.42&foo=not.eq.42 is false, and we allow the client to request that. It hasn't yet been our policy to disqualify tautologically false conditions. On one hand it's silly for the client to request it, but on the other hand the empty string is a possible value of .join(",") so I can see why it happens.
  2. We currently get two different behaviors for in. depending on whether the column is a string or number. When it's a number postgrest raises an error, but when it's a string then ?col=in. is equivalent to ?col=eq.. So we're already inconsistent and changing in. to mean "in the empty set" would resolve that. (Strictly speaking making it raise an error would resolve it too, but just wanted to point out it's inconsistent as is.)

@ruslantalpa
Copy link
Contributor

  1. again select * where id=10 and id != 10 is a valid query, select * where id in () is not, and raising the error is the correct thing.
  2. are you sure that is the case, looked at the haskell code and wrote the query by hand like so and it also is an error
    select * from test.projects where name in ("")

@daurnimator
Copy link
Contributor Author

again select * where id=10 and id != 10 is a valid query, select * where id in () is not, and raising the error is the correct thing.

In postgres id in (foo,bar) is just sugar for where id = foo or id = bar https://www.postgresql.org/docs/current/static/functions-comparisons.html#AEN20289
It's actually a bit weird they don't take the empty list.

@ruslantalpa
Copy link
Contributor

@daurnimator That might be true, but it's not for us mortals to judge :)
The only thing (in my opinion) from this thread that should be implemented if anyone wants to is to generate the query like so in () instead of in ("") so that the error is a bit more clear and points to the cause

@SebAlbert
Copy link

@ruslantalpa The query
select * from test.projects where name in ("")
is probably wrong because Postgres cares about " vs. ' and string literals are only enclosed in ', while " is reserved for identifiers (like ` in many other SQL implementations).

@ruslantalpa
Copy link
Contributor

Possible, i'll look at it again a bit later

On 16 Jun 2016, at 17:33, Sebastian Albert notifications@github.com wrote:

@ruslantalpa The query
select * from test.projects where name in ("")
is probably wrong because Postgres cares about " vs. ' and string literals are only enclosed in ', while " is reserved for identifiers (like ` in many other SQL implementations).


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@ruslantalpa
Copy link
Contributor

the request /projects?name=in. generates select * from projects where name in ('') which is valid. since we are talking about text column here, the part after . can be considered empty string, valid case ... and everything works.
the request /projects?id=in. generates id in ('') and the error at the top, i guess you could look at it as NULL and have id in (NULL)

the problem with this is that when generating the query, we would need to know beforehand the type of the column to switch '' to NULL but that will need a lot of changes, not sure it's worth it.

@begriffs
Copy link
Member

If someone wants to detect if name is the empty string they could still do ?name=eq.. What about using a special case for the in operator when given an empty array:

urlsql clause
in.1,2,3
in (1,2,3)
in.
false

Simply turn it to false in sql since we know that x in (), if it were valid sql, would have been false.

@ruslantalpa
Copy link
Contributor

ruslantalpa commented Jun 16, 2016

if it's valid sql and if it's one size fits all (column types) i'm for it

@begriffs
Copy link
Member

begriffs commented Jul 6, 2016

Yikes I've let this issue languish! I can work on the fix for this one. Adding a tag to the issue.

@begriffs begriffs added the bug label Jul 6, 2016
@diogob
Copy link
Contributor

diogob commented Jul 12, 2016

@begriffs @ruslantalpa @SebAlbert maybe we should think of the in. in terms of = any.

And since = any('{}') is valid and actually compares with an empty array the invalid syntax problem is no longer an argument for returning an error.

In fact, PostgreSQL even converts view definitions using IN to a different internal representation.

For example:

create view test_view as 
select * 
from pg_database 
where datname in ('template1', 'template2');

Gets created as:

postgres=# \d+ t5
...
View definition:
 SELECT pg_database.datname,
    pg_database.datdba,
    pg_database.encoding,
    pg_database.datcollate,
    pg_database.datctype,
    pg_database.datistemplate,
    pg_database.datallowconn,
    pg_database.datconnlimit,
    pg_database.datlastsysoid,
    pg_database.datfrozenxid,
    pg_database.datminmxid,
    pg_database.dattablespace,
    pg_database.datacl
   FROM pg_database
  WHERE pg_database.datname = ANY (ARRAY['template1'::name, 'template2'::name]);

steve-chavez added a commit to steve-chavez/postgrest that referenced this issue Apr 7, 2017
steve-chavez added a commit to steve-chavez/postgrest that referenced this issue Apr 7, 2017
steve-chavez added a commit to steve-chavez/postgrest that referenced this issue Apr 11, 2017
steve-chavez added a commit to steve-chavez/postgrest that referenced this issue Apr 11, 2017
monacoremo pushed a commit to monacoremo/postgrest that referenced this issue Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

5 participants