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

Feature Request: Ability to use cast syntax on order params #1651

Closed
gmichalec-pandora opened this issue Nov 16, 2020 · 7 comments
Closed

Feature Request: Ability to use cast syntax on order params #1651

gmichalec-pandora opened this issue Nov 16, 2020 · 7 comments

Comments

@gmichalec-pandora
Copy link

gmichalec-pandora commented Nov 16, 2020

Environment

  • PostgreSQL version: 11
  • PostgREST version: 7.0.1
  • Operating system: Debian Stretch (docker)

Description of issue

It seems that currently we aren't able to used casted fields in the order by param. For example, we have a field containing debian versions in string format like "8.10.1". Sorting these by string value won't return the desired result. However, we have a custom datatype 'debversion' that allows for proper sorting and comparison. However, attempting to order by a casted field returns an error.

(Expected behavior vs actual behavior)

Expected:

$ curl -H "Authorization: Bearer ${JWT}" "http://postgrest/order_test?select=textfield::int&order=textfield&limit=3"
[{"textfield":1}, 
 {"textfield":2}, 
 {"textfield":3}]

Actual:

$ curl -H "Authorization: Bearer ${JWT}" "http://postgrest/order_test?select=textfield::int&order=textfield::int&limit=3"
{"details":"unexpected ':' expecting letter, digit, \"-\", \"->>\", \"->\", delimiter (.), \",\" or end of input","message":"\"failed to parse order (textfield::int)\" (line 1, column 10)"}

(Steps to reproduce: Include a minimal SQL definition plus how you make the request to PostgREST and the response body)

create table order_test (textfield text);
insert into order_test values ('1'), ('2'), ('3'), ('4'), ('5'), ('6'), ('7'), ('8'), ('9'), ('10');
# without casted order
$ curl -H "Authorization: Bearer ${JWT}" "http://postgrest/order_test?select=textfield::int&order=textfield&limit=3"
[{"textfield":1}, 
 {"textfield":10}, 
 {"textfield":2}]

# with casted order
$ curl -H "Authorization: Bearer ${JWT}" "http://postgrest/order_test?select=textfield::int&order=textfield::int&limit=3"
{"details":"unexpected ':' expecting letter, digit, \"-\", \"->>\", \"->\", delimiter (.), \",\" or end of input","message":"\"failed to parse order (textfield::int)\" (line 1, column 10)"}

may be a separate issue, but i also noticed you can't cast on where clauses either:

# without casting:
$ curl -H "Authorization: Bearer ${JWT}" "http://postgrest/order_test?select=textfield::int&textfield=lt.3&order=textfield&limit=3"
[{"textfield":1}, 
 {"textfield":10}, 
 {"textfield":2}]
# with casting, but same results
$ curl -H "Authorization: Bearer ${JWT}" "http://postgrest/order_test?select=textfield::int&textfield::int=lt.3&order=textfield&limit=3"
[{"textfield":1}, 
 {"textfield":10}, 
 {"textfield":2}]
@gmichalec-pandora
Copy link
Author

gmichalec-pandora commented Nov 16, 2020

I should add, if anyone stumbles upon this, that a workaround now is to create a view with the casted field in the definition and select from that:

 create view order_test_view as select textfield::int from order_test;
 $ curl -H "Authorization: Bearer ${JWT}" "http://postgrest/order_test_view?select=textfield&order=textfield&limit=3"
 [{"textfield":1}, 
  {"textfield":2}, 
  {"textfield":3}]
 $ curl -H "Authorization: Bearer ${JWT}" "http://postgrest/order_test_view?select=textfield&textfield=lt.3&order=textfield&limit=3"
 [{"textfield":1}, 
  {"textfield":2}]

(and i realize the proper fix for this specific case would be to just alter the column type in the table schema - this is a contrived example)

@steve-chavez
Copy link
Member

may be a separate issue, but i also noticed you can't cast on where clauses either:

Ah, this is a something we're not likely to include, because casting the left operand in a comparison would invalidate index usage.

I wonder if that would be a concern for order as well?

that a workaround now is to create a view with the casted field in the definition and select from that:

Also, another option would be a computed/generated column.

CREATE FUNCTION os_release_debversion(os) RETURNS debversion AS $$
  SELECT $1.os_release_version::debversion;
$$ LANGUAGE SQL;

-- Then use like `GET /os?select=os_release_debversion&order=os_release_debversion

@gmichalec-pandora
Copy link
Author

gmichalec-pandora commented Nov 17, 2020

I can understand that argument, but that presumes there is an index on the field at all - and in this case there is not. I'd argue that sometimes convenience sometimes trumps efficiency, and it's up to a user to know whether they will be triggering a slow query.
I don't know what goes on in the postgrest backend, but I assume that if I did create an index on textfield::int, using the casting on the where or order clauses would use that index, just like it would for a regular query?

@wolfgangwalther
Copy link
Member

wolfgangwalther commented Nov 18, 2020

I don't know what goes on in the postgrest backend, but I assume that if I did create an index on textfield::int, using the casting on the where or order clauses would use that index, just like it would for a regular query?

My understanding is that, when we do column = 'value'::unknown (the default without casting), there's only one implicit cast that can happen - the one to the column type. This allows index usage. Assuming column is not of type int, if we do column = 'value'::int (or any other cast) instead, there might be multiple implicit casts that are possible. If the wrong one is chosen, indexes can't be used anymore.

The solution proposed by Steve is great. A simple one-liner SQL function like this will be inlined, so this should give exactly the result you want, without the need to create a view just for that. And it allows for much more complex scenarios as well.

@wolfgangwalther
Copy link
Member

If for whatever reason the suggested solution does not work or there are other arguments in favor of the cast, feel free to reopen. For now, this seems settled, so I'm closing this.

@steve-chavez
Copy link
Member

I assume that if I did create an index on textfield::int, using the casting on the where or order clauses would use that index, just like it would for a regular query?

@gmichalec-pandora Has a point here. And that is indeed true, a CAST can be indexed and it would work through PostgREST.

and it's up to a user to know whether they will be triggering a slow query

That is the problem though. External API users don't necessarily know that, and malicious users could take advantage of unindexed casting to put a heavy load on your db.

We'd need an strict mode(maybe enforcing that max-rows is set) to enable this sort of flexibility.

@steve-chavez
Copy link
Member

Had another request for this:

How do I use order for JSONB fields with casts, like this order by clause
ORDER BY (info->>'createdAt')::timestamptz

The computed column way still applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants