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

Fixes Oracle LIMIT and adds ROracle support #27

Merged
merged 10 commits into from Oct 24, 2017

Conversation

edgararuiz-zz
Copy link
Contributor

@edgararuiz-zz edgararuiz-zz commented Jul 4, 2017

Fixes #tidyverse/dplyr#2928

  1. An issue was discovered while working out how to add ROracle support. The clause used currently to perform the LIMIT is only available for Oracle version 12c and is not backwards compatible. The alternative is to a technique found here: https://oracle-base.com/articles/misc/top-n-queries. We need to limit the rows in a WHERE clause, so had to use a subquery to fix.

  2. Mapped all of the methods from the Oracle to the ROracle class (OraConnection). I also mapped some OdbcConnection methods as an attempt to provide as much functionality as possible, meaning copy_to() should work now.

Copy link
Member

@hadley hadley left a comment

And can you please add a bullet to NEWS?

@@ -21,11 +21,6 @@ test_that("as.numeric() translated to NUMBER ", {
df <- data.frame(x = 1, y = 2)
df_oracle <- tbl_lazy(df, src = simulate_oracle())

test_that("query uses FETCH FIRST x ROWS instead of LIMIT ", {
Copy link
Member

@hadley hadley Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update this test please?

@edgararuiz-zz
Copy link
Contributor Author

@edgararuiz-zz edgararuiz-zz commented Jul 5, 2017

So far, so good : tidyverse/dplyr#2928 (comment)

@bthe
Copy link

@bthe bthe commented Sep 25, 2017

Any news on this pull request?

R/db-roracle.R Outdated

#' @export
db_analyze.OraConnection <- function(con, table, ...) {
db_analyze.Oracle(con = con, table = table)
Copy link
Member

@hadley hadley Oct 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please pass ... down through all these functions? That keeps it forward compatible

@hadley hadley merged commit 7b45148 into tidyverse:master Oct 24, 2017
1 check was pending
@hadley
Copy link
Member

@hadley hadley commented Oct 24, 2017

Thanks @edgararuiz!

@thomastallaksen
Copy link

@thomastallaksen thomastallaksen commented Feb 25, 2021

The fix for backwards compatibility with Oracle 11g no longer works in newer versions of dbplyr.

@fermumen
Copy link

@fermumen fermumen commented Apr 30, 2021

I can confirm what @thomastallaksen is saying, the newest versions revert to FETCH FIRST ... ROWS ONLY that only works on 12c or later. It should be using WHERE rownum < 6

@hadley
Copy link
Member

@hadley hadley commented Apr 30, 2021

I don't have the resources to support very old versions of Oracle, and so dbplyr is only compatible with versions that have still extended support. Extended support for 11.2 finished in Dec 2020.

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.

None yet

6 participants