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.

Sign up for GitHub

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

Jump to bottom

order Supports Postgres JSON operators #43020

Closed
wants to merge 1 commit into from
Closed

order Supports Postgres JSON operators #43020

wants to merge 1 commit into from

Conversation

ChaelCodes
Copy link
Contributor

Summary

closes #43014

Today, when Postgres JSON operators are used, they are treated as potentially user input, and filtered out to protect against SQL injection attacks. There is a regex pattern that is used to check if the string is a column, or function wrapping a column. This regex is already separated into the postgres adapter.

This PR adds one operator (->> column_name) so that order works more like where clauses do.

There's also documentation added to the JSON guide about ordering using JSON operators

Other Information

Postgres supports 3 different JSON operators, but we only support one. Here's why.

Operator Description Example Why?
-> Get JSON array element '[1,2,3]'::json->2 returns unorderable field
-> Get JSON object field '{"a":1,"b":2}'::json->'b' returns unorderable field
->> Get JSON array element as text '[1,2,3]'::json->>2 Supported!
->> Get JSON object field as text '{"a":1,"b":2}'::json->>'b' Supported!
#> Get JSON object at specified path '{"a":[1,2,3],"b":[4,5,6]}'::json#>'{a,2}' returns unorderable field
#>> Get JSON object at specified path as text '{"a":[1,2,3],"b":[4,5,6]}'::json#>>'{a,2}' Let's not support nested json

@ChaelCodes ChaelCodes force-pushed the cc-support-postgres-json-operators branch from 5cc29ba to 14389ff Compare August 15, 2021 21:10
@p8
Copy link
Member

p8 commented Aug 16, 2021

Hi @ChaelCodes,
Thanks for the detailed PR!
Could you add a Changelog entry as well?

@ChaelCodes ChaelCodes force-pushed the cc-support-postgres-json-operators branch 2 times, most recently from f1de9ac to 4ef046a Compare August 16, 2021 19:59
@ChaelCodes
Copy link
Contributor Author

@p8 Added a Changelog entry! There was a misspelling in the changelog previously, so I fixed that so CI would pass.

@p8
Copy link
Member

p8 commented Aug 16, 2021

@ChaelCodes great!
Hopefully someone with more experience with Postgres than myself will have a look at the implementation soon. 🙂

@ChaelCodes ChaelCodes force-pushed the cc-support-postgres-json-operators branch from c028557 to 4824f4e Compare August 17, 2021 17:01
@JuanVqz
Copy link
Contributor

JuanVqz commented Aug 17, 2021

Hi, @ChaelCodes I've seen you are doing contributions here, will be great if you can do it in streaming. I'd love to do it too. but it's hard to get to a point where start.

@ChaelCodes
Copy link
Contributor Author

Hi @JuanVqz, I streamed working on this PR on Sunday. I try to maintain only one PR on busy repos like Rails, and respond quickly to review feedback. This means that I work on a Rails PR every couple of months, and rarely share the feedback and review process, but I do share their initial build-out.

simi
simi reviewed Aug 29, 2021
View reviewed changes
activerecord/CHANGELOG.md Outdated Show resolved Hide resolved
@ChaelCodes
Allow Postgres JSON operator to be used in order
2874d02 
Previously, any JSON operator would return the error
`ActiveRecord::UnknownAttributeReference`. This error is intended to
protect against SQL injection. It requires the developer to wrap the
query in `Arel.sql` to avoid the error. Referencing a JSON column should
be treated the same as referencing a string column.

This commit only allows `->>` to be used in order.
@ChaelCodes ChaelCodes force-pushed the cc-support-postgres-json-operators branch from 4824f4e to 2874d02 Compare September 5, 2021 15:20
@key88sf
Copy link

key88sf commented Oct 21, 2021

@ChaelCodes Also getting this error ActiveRecord::UnknownAttributeReference when trying to order by a Postgresql function like this:
User.order( "levenshtein(firstname, 'Alicia')" )

It seems like if there is any function that takes multiple arguments in the order-by string clause, it fails with this error.
Here's a generic example:
User.order( "myfunc(x,y)" ) # ActiveRecord::UnknownAttributeReference

Not sure if your PR addresses this case or not? Wanted to point it out.

Ideally it would be great to be able to do:

User.order( "myfunc(?, ?)", x, y ) # Substitute the value of x and y for the question marks like in a where clause

@key88sf key88sf mentioned this pull request Oct 21, 2021
Open
@rails-bot
Copy link

rails-bot bot commented Jan 19, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Jan 19, 2022
@rails-bot rails-bot bot closed this Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simi simi simi left review comments

@hahmed hahmed hahmed left review comments

Assignees
No one assigned
Labels
activerecord docs stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ActiveRecord raises an ActiveRecord::UnknownAttributeReference error on order of a PostgreSQL json column
6 participants
@ChaelCodes @p8 @JuanVqz @key88sf @simi @hahmed