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

ActiveRecord 6.1 compatibility #109

Merged
merged 9 commits into from
Nov 15, 2021
Merged

ActiveRecord 6.1 compatibility #109

merged 9 commits into from
Nov 15, 2021

Conversation

tagliala
Copy link
Member

@tagliala tagliala commented Jan 15, 2021

Bringing AR 6.1 compatibility require more work, because Rails 6.1 supports multiple databases and that required a complete rewrite of the database configuration

The method used by Chrono Model to implement rake tasks, current_config, is deprecated and will be removed in 6.2, but changing the rake tasks should not be a huge deal

Also, some schema related are deprecated and will be removed in Rails 6.2

TODO:

  • Fix data dump
  • Fix add_index and remove_index with Ruby 3.x
  • Fix other failing specs

@tagliala tagliala force-pushed the feature/ar-61 branch 6 times, most recently from 2e49472 to 78c5901 Compare October 12, 2021 14:26
@tagliala tagliala force-pushed the feature/ar-61 branch 7 times, most recently from 594669a to 2ee03d6 Compare November 6, 2021 00:37
@tagliala
Copy link
Member Author

tagliala commented Nov 6, 2021

relation = relation.joins(*assocs.map(&:name))

is generating a different query on Rails 6.1

Bisected to rails/rails#40540

https://github.com/rails/rails/pull/40540/files#diff-74b02568dc5480338f058cd75ca47142eb7f611c456bdc21992f06b7c6994e55L72-R73


6.1

> self.joins(:foo).to_sql
"SELECT \"history\".\"bars\".* FROM \"history\".\"bars\" INNER JOIN (SELECT \"history\".\"foos\".* FROM \"history\".\"foos\" WHERE ( '2021-11-06 12:31:29.796839'::timestamp <@ history.foos.validity )) \"foos\" ON \"foos\".\"id\" = \"history\".\"bars\".\"foo_id\""

> self.except(:order).joins(:foo).to_sql
"SELECT \"history\".\"bars\".* FROM \"history\".\"bars\" INNER JOIN (SELECT \"history\".\"foos\".* FROM \"history\".\"foos\" WHERE ( '2021-11-06 12:29:40.275442'::timestamp <@ history.foos.validity )) \"foos\" ON \"foos\".\"id\" = \"history\".\"bars\".\"foo_id\""

6.0

> self.joins(:foo).to_sql
"SELECT \"history\".\"bars\".* FROM \"history\".\"bars\" INNER JOIN (SELECT \"history\".\"foos\".* FROM \"history\".\"foos\" WHERE ( '2021-11-06 12:32:04.216929'::timestamp <@ history.foos.validity )) \"foos\" ON \"foos\".\"id\" = \"history\".\"bars\".\"foo_id\""

> self.except(:order).joins(:foo).to_sql
"SELECT \"history\".\"bars\".* FROM \"history\".\"bars\" INNER JOIN \"foos\" ON \"foos\".\"id\" = \"history\".\"bars\".\"foo_id\""

except(:order) is dropping SELECT \"history\".\"foos\" part in < 6.1

@tagliala
Copy link
Member Author

@gridanjbf any chance to squash bf3addf and cfa3c7f and push force a single commit?

@tagliala tagliala changed the title Feature/ar 61 ActiveRecord 6.1 compatibility Nov 10, 2021
@gridanjbf
Copy link
Contributor

@gridanjbf any chance to squash bf3addf and cfa3c7f and push force a single commit?

i will do a reset --soft and then commit

@gridanjbf gridanjbf force-pushed the feature/ar-61 branch 2 times, most recently from 0c4fa45 to c098eb5 Compare November 10, 2021 13:04
lib/chrono_model/time_machine/timeline.rb Show resolved Hide resolved
lib/chrono_model/time_machine/timeline.rb Outdated Show resolved Hide resolved
lib/chrono_model/time_machine/timeline.rb Show resolved Hide resolved
lib/chrono_model/time_machine/timeline.rb Show resolved Hide resolved
lib/chrono_model/adapter/migrations.rb Outdated Show resolved Hide resolved
lib/chrono_model/adapter/migrations.rb Outdated Show resolved Hide resolved
lib/chrono_model/adapter/migrations.rb Outdated Show resolved Hide resolved
@tagliala
Copy link
Member Author

We can now drop efdd9c7

tagliala and others added 8 commits November 15, 2021 18:46
TODO: remove this commit
In the `else` case we should keep `ass.foreign_key` because we are
in the `has_many` case. foo has many bars and we are in :bars
association.

`association.foreign_key` returns `foo_id`,
`association.association_foreign_key` returns `:bar_id` because it does
`Bar.foreign_key`
@tagliala tagliala marked this pull request as ready for review November 15, 2021 17:50
@tagliala tagliala merged commit 076e870 into master Nov 15, 2021
@tagliala tagliala deleted the feature/ar-61 branch November 15, 2021 17:51
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.

3 participants