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

PostgreSQL bugfix for invalid SQL in subqueries #243

Merged
merged 2 commits into from
Feb 5, 2014
Merged

PostgreSQL bugfix for invalid SQL in subqueries #243

merged 2 commits into from
Feb 5, 2014

Conversation

vanderhoorn
Copy link

In commit 68a9554 the last_column feature of ToSql was removed. The visit_Arel_Nodes_Matches and visit_Arel_Nodes_DoesNotMatch methods are overwritten in the PostgreSQL class, but were not updated appropriately. This commit fixes the issue accordingly.

This bug affects at least all update_all statements in Rails 4.0.2 that have subqueries with ILIKE statements on PostgreSQL. The bug is present in Arel 4.0.1 and later, so it probably affects most Rails 4.0.2 projects.

It would be highly appreciated if Arel 4 could get a point release as well. Thanks for your continued work.

In commit 68a9554 the last_column feature of ToSql was removed. The visit_Arel_Nodes_Matches and visit_Arel_Nodes_DoesNotMatch methods are overwritten in the PostgreSQL class, but were not updated appropriately. This commit fixes the issue accordingly.

This bug affects at least all update_all statements in Rails 4.0.2 that have subqueries with ILIKE statements on PostgreSQL. The bug is present in Arel 4.0.1 and later, so it probably affects most Rails 4.0.2 projects.

It would be highly appreciated if Arel 4 could get a point release as well. Thanks for your continued work.
@rafaelfranca
Copy link
Member

@vanderhoorn thank you so much for the contribution. Could you create a test case for this failure? With this we can make sure it will not break again in the future.

@vanderhoorn
Copy link
Author

Well, currently there aren't any tests for the visit_Arel_Nodes_Matches and visit_Arel_Nodes_DoesNotMatch methods; not in the PostgreSQL class and not in the ToSql class.

The code in https://gist.github.com/vanderhoorn/8824862 (written by my colleague @StefanH) reproduces the issue. You need a PostgreSQL database named rails-arel-update_all-bug to be able to run the code.

@ernie
Copy link
Collaborator

ernie commented Feb 5, 2014

@vanderhoorn Yeah, that's how we accidentally introduced the bug in the first place. Definitely would help to have a test added.

@vanderhoorn
Copy link
Author

Done, added tests.

As a side-note, @ernie, you may want to blame the original author for not adding tests in the first place ;)

@ernie
Copy link
Collaborator

ernie commented Feb 5, 2014

Hey, the tests passed for me! :P

@vanderhoorn
Copy link
Author

If someone could make a point release for the 4.x branch, that would be really amazing.

We maintain the ancestry gem (https://github.com/stefankroes/ancestry). The master branch has some queries changed to use Arel instead of self-written SQL. We cannot release a new version of ancestry, without this bugfix, as it would break many Rails 4.0.2 applications that use ancestry.

@vanderhoorn
Copy link
Author

Or should I do a commit to the 4-0-stable branch as well?

rafaelfranca added a commit that referenced this pull request Feb 5, 2014
PostgreSQL bugfix for invalid SQL in subqueries
@rafaelfranca rafaelfranca merged commit e8aa045 into rails:master Feb 5, 2014
rafaelfranca added a commit that referenced this pull request Feb 5, 2014
PostgreSQL bugfix for invalid SQL in subqueries
@rafaelfranca
Copy link
Member

4.0.2 released

@vanderhoorn
Copy link
Author

Super! Thanks!

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

Successfully merging this pull request may close these issues.

3 participants