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

The new version is not compatible with Oracle #908

Closed
apalacio9502 opened this issue Jun 6, 2022 · 4 comments · Fixed by #917
Closed

The new version is not compatible with Oracle #908

apalacio9502 opened this issue Jun 6, 2022 · 4 comments · Fixed by #917

Comments

@apalacio9502
Copy link

Hello,

The modification in this commit 8dd7639 is very interesting, however in the case of Oracle it is not fully functional.

In Oracle, if I want to select all the columns and add a new one, I must write the query in the following way
SELECT q01.*, COLUM_1+COLUM_2 AS VALUE FROM TEST q01;

And not as dbplyr is currently doing, which is this way.

SELECT *, COLUM_1+COLUM_2 AS VALUE FROM TEST q01;

The big difference is that the * is preceded by the name that the table is being called, which in this case is q01

Thank you.

@mgirlich
Copy link
Collaborator

@apalacio9502 I have added a PR that hopefully fixes the issues with Oracle. It would be great if you could install it via

remotes::install_github(
  "tidyverse/dbplyr",
  ref = "oracle-compat"
)

and give feedback whether this solves your dbplyr issues with Oracle.

@eipi10
Copy link
Contributor

eipi10 commented Jun 20, 2022

@mgirlich I got an error querying an Oracle database this morning after upgrading dbplyr to the latest version (see below for an example of the error I'm getting). I came here and found this issue. After testing, I can confirm that the "oracle-compat" PR fixes the error.

For reference, here is the error:

Error: nanodbc/nanodbc.cpp:1655: HY000: [Oracle][ODBC][Ora]ORA-00923: FROM keyword not found where expected
 
<SQL> 'SELECT *, "emplid" || ' ' || "admit.type" AS "admit.type.new"
FROM (
  SELECT
    "EMPLID" AS "emplid",
    "EMAILID" AS "emailid",
    "NAME" AS "name",
    "ADMIT_TYPE" AS "admit.type",
    "ENDDTTM" AS "enddttm"
  FROM ("UDW_STU_DMT"."STG_PS_SAC_FIF_TBL") 
) "q01"
FETCH FIRST 11 ROWS ONLY'

And here is the SQL generated after installing the "oracle-compat" PR:

<SQL>
SELECT
  "emplid",
  "emailid",
  "name",
  "admit.type",
  "enddttm",
  "emplid" || ' ' || "admit.type" AS "new"
FROM (
  SELECT
    "EMPLID" AS "emplid",
    "EMAILID" AS "emailid",
    "NAME" AS "name",
    "ADMIT_TYPE" AS "admit.type",
    "ENDDTTM" AS "enddttm"
  FROM ("UDW_STU_DMT"."STG_PS_SAC_FIF_TBL") 
) "q01"

@eipi10
Copy link
Contributor

eipi10 commented Jun 21, 2022

I spoke too soon. If I use paste to unite two columns, the code works. However, if I use str_c to unite two columns, I get an error. I know that paste worked with previous versions of dbplyr, however, I'm not sure if the str_c version ever worked. Here is the SQL generated with the "oracle-compat" branch installed.

The following is a query generated using mutate(new=paste(admit.type, emplid)). It runs as expected:

<SQL>
SELECT
  "emplid",
  "emailid",
  "name",
  "admit.type",
  "enddttm",
  "admit.type" || ' ' || "emplid" AS "new" 
FROM (
  SELECT
    "EMPLID" AS "emplid",
    "EMAILID" AS "emailid",
    "NAME" AS "name",
    "ADMIT_TYPE" AS "admit.type",
    "ENDDTTM" AS "enddttm"
  FROM ("UDW_STU_DMT"."STG_PS_SAC_FIF_TBL") 
) "q01"

The SQL below was generated using mutate(new=str_c(admit.type, emplid)). It generates the following error:
Error: nanodbc/nanodbc.cpp:1655: 42S22: [Oracle][ODBC][Ora]ORA-00904: "CONCAT_WS": invalid identifier.

<SQL>
SELECT
  "emplid",
  "emailid",
  "name",
  "admit.type",
  "enddttm",
  CONCAT_WS('', "admit.type", "emplid") AS "new"
FROM (
  SELECT
    "EMPLID" AS "emplid",
    "EMAILID" AS "emailid",
    "NAME" AS "name",
    "ADMIT_TYPE" AS "admit.type",
    "ENDDTTM" AS "enddttm"
  FROM ("UDW_STU_DMT"."STG_PS_SAC_FIF_TBL") 
) "q01"

@mgirlich
Copy link
Collaborator

@eipi10 I'm pretty sure that str_c() never worked for Oracle as the code wasn't touched. I added a separate PR #921 for this.
If you find other functions not being translated correctly, you could help us by

  • providing SQL how a correct translation should look like
  • or even greater by creating a PR

Thanks a lot for testing out the compat PR 👍

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 a pull request may close this issue.

3 participants