-
Notifications
You must be signed in to change notification settings - Fork 309
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 columns_for_distinct when using Rails 6.1 #2249
Conversation
Running `ARCONN=oracle bundle exec ruby -Itest ./test/cases/finder_test.rb -n test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct` in Rails 6.1.4.4 will result in an error. ``` ARCONN=oracle bundle exec ruby -Itest ./test/cases/finder_test.rb -n test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct Using oracle Warning: NLS_LANG is not set. fallback to US7ASCII. Run options: -n test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct --seed 32511 # Running: F Failure: FinderTest#test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct [./test/cases/finder_test.rb:1503]: Expected: 2 Actual: 1 rails test ./test/cases/finder_test.rb:1502 Finished in 1.275346s, 0.7841 runs/s, 0.7841 assertions/s. 1 runs, 1 assertions, 1 failures, 0 errors, 0 skips ``` Changing the return value of `columns_for_distinct` causes the test to pass. This is a fix for the change to PostgreSQL and MySQL adapters in Rails 6.0. rails/rails#31966 The reason why the tests for `ARCONN=oracle bundle exec ruby -Itest ./test/cases/finder_test.rb -n test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct` don't pass is because of a fix in finder_methods.rb in Rails 6.1. rails/rails@2ad2425
Thanks for opening a pull request. Would you make sure this test fails against Rails main branch (7.1.0.alpha) and Oracle enhanced adapter master branch? If so, I'm happy to merge this pull request and backport them to release70 and release61 branches. |
Tests failed on the Rails main branch and Oracle enhanced adapter master branch. |
Thank you for the confirmation. Let me backport it to release70 and release61 branches. |
Merge pull request #2249 from swamp09/fix_columns_for_distinct
Merge pull request #2249 from swamp09/fix_columns_for_distinct
Running
ARCONN=oracle bundle exec ruby -Itest ./test/cases/finder_test.rb -n test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct
in Rails 6.1.4.4 will result in an error.Fixing the return value of
columns_for_distinct
causes the test to pass.This is a fix for the change to PostgreSQL and MySQL adapters in Rails 6.0. rails/rails#31966
The reason why the tests for
ARCONN=oracle bundle exec ruby -Itest ./test/cases/finder_test.rb -n test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct
don't pass is because of a fix in finder_methods.rb in Rails 6.1.rails/rails@2ad2425