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 an AR test of relations_test when using Oracle #28721

Merged

Conversation

koic
Copy link
Contributor

@koic koic commented Apr 10, 2017

Summary

This PR fixes the following failure test when using Oracle (11g) .

%  ruby -v
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
% ARCONN=oracle bundle exec ruby -w -Itest test/cases/relations_test.rb -n "test_relations_don't_load_all_records_in_#inspect"
/home/vagrant/src/rails/activesupport/lib/active_support/core_ext/enumerable.rb:20: warning: method redefined; discarding old sum
Using oracle
Run options: -n test_relations_don't_load_all_records_in_#inspect --seed 24645

# Running:

F

Finished in 0.994688s, 1.0053 runs/s, 1.0053 assertions/s.

  1) Failure:
RelationTest#test_relations_don't_load_all_records_in_#inspect [test/cases/relations_test.rb:1907]:
Query pattern(s) /LIMIT/ not found.
Queries:
SELECT  "POSTS".* FROM "POSTS" WHERE ROWNUM <= :a1

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips

Oracle does not support LIMIT clause. Oracle Adapter uses ROWNUM instead of that.

Other Information

I confirmed that there is the same test fails on 5-1-stable branch.

@rails-bot
Copy link

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

@yahonda
Copy link
Member

yahonda commented Apr 10, 2017

@koic Thanks for opening a pull request. Would you include Oracle 12c new syntax fetch first n rows only.

$ ARCONN=oracle bundle exec ruby -w -Itest test/cases/relations_test.rb -n "test_relations_don't_load_all_records_in_#inspect"
/home/yahonda/git/rails/activesupport/lib/active_support/core_ext/enumerable.rb:20: warning: method redefined; discarding old sum
Using oracle
Run options: -n test_relations_don't_load_all_records_in_#inspect --seed 41532

# Running:

F

Finished in 1.202606s, 0.8315 runs/s, 0.8315 assertions/s.

  1) Failure:
RelationTest#test_relations_don't_load_all_records_in_#inspect [test/cases/relations_test.rb:1905]:
Query pattern(s) /LIMIT/ not found.
Queries:
SELECT  "POSTS".* FROM "POSTS" FETCH FIRST :a1 ROWS ONLY

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
$

There was a similar pull request #26067.

@koic koic force-pushed the oracle_does_not_support_limit_clause branch from fefdc1e to 66cc3c7 Compare April 11, 2017 06:45
@koic
Copy link
Contributor Author

koic commented Apr 11, 2017

@yahonda Thanks for your advice. I've included an Oracle 12c new syntax FETCH FIRST n ROWS ONLY with reference to your advice.

@yahonda
Copy link
Member

yahonda commented Apr 11, 2017

Thanks for updating the pull request.

Now this test passes with Oracle Database 12c

$ ARCONN=oracle bundle exec ruby -w -Itest test/cases/relations_test.rb -n "test_relations_don't_load_all_records_in_#inspect"
/home/yahonda/git/rails/activesupport/lib/active_support/core_ext/enumerable.rb:20: warning: method redefined; discarding old sum
Using oracle
Run options: -n test_relations_don't_load_all_records_in_#inspect --seed 56674

# Running:

.

Finished in 1.128852s, 0.8859 runs/s, 0.8859 assertions/s.

1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
$

@rafaelfranca rafaelfranca merged commit e426bc2 into rails:master Apr 11, 2017
@koic koic deleted the oracle_does_not_support_limit_clause branch April 11, 2017 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants