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

Fix order option with arel nodes #79

Merged
merged 3 commits into from
Mar 6, 2019

Conversation

felixbuenemann
Copy link
Collaborator

The current code uses .inspect within eval, so it only works if inspect returns the actual ruby representation of the code, it works with stuff like nil or {id: :desc}, but completely breaks for things like Arel.sql('"foo" DESC, "bar" ASC').

Fixes #78

This fixes:

> ActiveRecord::StatementInvalid: SQLite3::ConstraintException:
> UNIQUE constraint failed: schema_migrations.version:
> INSERT INTO "schema_migrations" (version) VALUES ('1')
@felixbuenemann
Copy link
Collaborator Author

Also pushed a change that should fix the following exception in the test suite, which is caused by newer sqlite versions that support UNIQUE contraints:

ActiveRecord::StatementInvalid: SQLite3::ConstraintException: UNIQUE constraint failed: schema_migrations.version: INSERT INTO "schema_migrations" (version) VALUES ('1')

this is technically not needed, but we would otherwise need to specify a
compatible version for each rails major in gemfiles and all versions
including 6.x support 1.3.6+ so iot should be fine for now.
@felixbuenemann
Copy link
Collaborator Author

Looks like only Rails 3.0 and 5.2+ work with sqlite 1.4, so to avoid touching all the gemfiles, I followed the example from #78 and locked down to ~> 1.3.6 in the gemspec dev dependency.

@felixbuenemann felixbuenemann merged commit 2733302 into master Mar 6, 2019
@felixbuenemann felixbuenemann deleted the support-order-by-arel-nodes branch March 7, 2019 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant