OHDSI / Achilles

Automated Characterization of Health Information at Large-scale Longitudinal Evidence Systems (ACHILLES) - descriptive statistics about a OMOP CDM database
https://ohdsi.github.io/Achilles/
130 stars 121 forks source link

Analysis 2004 fails on Oracle 19 #706

Closed tlecarrour-ee closed 1 year ago

tlecarrour-ee commented 1 year ago

Hi,

Describe the bug Analysis 2004 fails on Oracle 19 with message ORA-00907: missing right parenthesis.

To Reproduce Run the analysis on Oracle 19 using Achilles v1.7.0.

Possible fix Looks like the as command does not work as expected on Oracle 19. Removing them in some places fixes the problem. For instance:

  (SELECT count(distinct(person_id)) as totalPersons FROM omop_cdm.person ) as totalPersonsDb   UNION ALL

As to be replaced with:

  (SELECT count(distinct(person_id)) as totalPersons FROM omop_cdm.person ) totalPersonsDb   UNION ALL
fdefalco commented 1 year ago

Specific database platform support is controlled by the SqlRender package. The issue would need to be addressed in SqlRender in order to resolve within Achilles. Suggest that you ensure you are using the latest version of SqlRender or post an issue on the OHDSI/SqlRender package.

tlecarrour-ee commented 1 year ago

Thanks for your reply! I was not sure were to report this. I'll update my SqlRender to the latest version and open an issue there if the problem persists.

schuemie commented 1 year ago

I think this query isn't very pretty. Here's the first part of the big union, after running it through a SQL formatter:

SELECT 2004 AS analysis_id,
        CAST('0000001' AS VARCHAR(255)) AS stratum_1,
        CAST((1.0 * personIntersection.count_value / totalPersonsDb.totalPersons) AS VARCHAR(255)) AS stratum_2,
        CAST(NULL AS VARCHAR(255)) AS stratum_3,
        CAST(NULL AS VARCHAR(255)) AS stratum_4,
        CAST(NULL AS VARCHAR(255)) AS stratum_5,
        personIntersection.count_value
    FROM (
        SELECT COUNT(*) AS count_value
        FROM (
            SELECT person_id
            FROM #obs
            ) AS subquery
        ) AS personIntersection,
        (
            SELECT COUNT(DISTINCT (person_id)) AS totalPersons
            FROM @cdmDatabaseSchema.person
            ) AS totalPersonsDb

    UNION ALL

But most relevant to the issue, it uses 'AS' for table aliasing, which, as stated in the SqlRender vignette should be avoided.

Could this be fixed in ACHILLES? There's nothing I can do about it on the SqlRender side.

fdefalco commented 1 year ago

I've committed a change to the develop branch that should fix this issue. It ran successfully against our test Oracle infrastructure however it would be helpful to have confirmation from @tlecarrour-ee . Could you pull from the develop branch and confirm the changes resolve your issue?

tlecarrour-ee commented 1 year ago

Thanks for the fix @fdefalco! I've tried and failed to install the develop version. I get:

Error: package or namespace load failed for 'Achilles' in namespaceExport(ns, exports):
 undefined exports: generateDbSummary

I install it the same way I install the stable versions with devtools::install_github("OHDSI/Achilles@develop").

fdefalco commented 1 year ago

I believe that was an issue with an export that has been resolved. Can you test the current develop branch in your environment again?

tlecarrour-ee commented 1 year ago

Hi @fdefalco

Problem solved! 👍 Thanks!

2023-03-27 09:41:16[Main thread] INFO AchillesachillesAnalysis 2004 (Number of distinct patients that overlap between specific domains) -- START
2023-03-27 09:46:55[Main thread] INFO DatabaseConnector executeSql Executing SQL took 5.65 mins
2023-03-27 09:46:55[Main thread] INFO Achillesachilles [Main Analysis] [COMPLETE] 2004 (5.646201 mins)