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

Use FETCH FIRST for Oracle12 and test ROWNUM <= for Oracle 11g or older version to test sql limit behavior #26067

Merged
merged 1 commit into from Aug 8, 2016

Conversation

yahonda
Copy link
Member

@yahonda yahonda commented Aug 5, 2016

Summary

This pull request addresses this following failure when tested with Oracle Database 12c.

$ cd activerecord
$ ARCONN=oracle bundle exec ruby -W -w -I"lib:test" test/cases/finder_test.rb -n test_take_and_first_and_last_with_integer_should_use_sql_limit

... snip ...

# Running:

F

Finished in 0.826707s, 1.2096 runs/s, 1.2096 assertions/s.

  1) Failure:
FinderTest#test_take_and_first_and_last_with_integer_should_use_sql_limit [test/cases/finder_test.rb:568]:
Query pattern(s) /LIMIT|ROWNUM <=/ not found.
Queries:
SELECT  "TOPICS".* FROM "TOPICS" FETCH FIRST :a1 ROWS ONLY

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

Since Oracle 12c database introduces better top N query using FETCH FIRST n ROWS and Arel supports this syntax

Other Information

* Rails `master` branch
* Oracle enhanced adapter `rails51` branch
* Oracle Database 12c Enterprise Edition Release 12.1.0.2.0 - 64bit Production
* ruby 2.4.0dev (2016-08-04 trunk 55812) [x86_64-linux]

@rails-bot
Copy link

r? @matthewd

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

@yahonda
Copy link
Member Author

yahonda commented Aug 6, 2016

Added another similar case and squashed.

@senny
Copy link
Member

senny commented Aug 8, 2016

@yahonda should we check for both or are previous versions not compatible with Rails 5?

@yahonda
Copy link
Member Author

yahonda commented Aug 8, 2016

@senny Thanks for the review.
Old behaviour is still compatible with Rails 5. Let me update my commits.

@yahonda yahonda changed the title Use FETCH FIRST instead of ROWNUM <= for limit Use FETCH FIRST for Oracle12 and test ROWNUM <= for Oracle 11g or older version to test sql limit behavior Aug 8, 2016
also test `ROWNUM <=` for Oracle 11g or older version of Oracle and Oracle visitor
Oracle 12c database and Arel Oracle12 visitor supports better top N query.
@yahonda
Copy link
Member Author

yahonda commented Aug 8, 2016

Add Oracle 11g support then squashed. Somehow it gets an error at ActionCable https://travis-ci.org/rails/rails/jobs/150643333

@senny senny merged commit d8fe252 into rails:master Aug 8, 2016
@senny
Copy link
Member

senny commented Aug 8, 2016

@yahonda thanks. I'll backport it to 5-0-stable as well.

senny added a commit that referenced this pull request Aug 8, 2016
Use `FETCH FIRST` for Oracle12 and test `ROWNUM <=` for Oracle 11g or older version to test sql limit behavior
@yahonda yahonda deleted the oracle_can_use_fetch_first branch December 9, 2016 21:20
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

5 participants