Fix does_not_support_reverse? to find sql functions with commas in nested brackets #25987
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
does_not_support_reverse?
is supposed to check if a query statement is too complex to reverse its order. One of the scenarios is if there is a sql function with multiple arguments, by using/\([^()]*,[^()]*\)/
to match. However, it doesn't catch the case when a query statement has nested parentheses. For instance,concat(author_name, lower(title))
is regarded as supported to do reverse, andUser.order(concat(author_name, lower(title))).last
will raise a sql syntax or undefined column error that is quite confusing, because it does try to reverse the orderOther Information
The new implementation is a bit longer but is not necessarily slower. Here's the benchmark for order statements in different complexity levels: