Unable to generate cohorts: BadSqlGrammer error

PRijnbeek commented 6 years ago

Expected behavior

Succes

Actual behavior

Failed.

I have a multi database setup that worked with previous WebApi and Atlas

Error message:

error.txt

Steps to reproduce behavior

Running on Postgres. Cohort (json):

cohort.txt

How can I best proceed to find the issue? can I get the actual full sql that is executed from somewhere? the error field in cohort_generation_info has a max length that does not allow the full error message. Maybe we should increase this field size? Would it help to manually do this niw or is the text length also limited somewhere else in the code?

Are there maybe new tables that need to be added in the individual database schemas that I am not aware of?

chrisknoll commented 6 years ago

@PRijnbeek , I extracted the cohort sql via atlas into the attached txt file. Would you be able to run it (after replacing the tokens with your schema names) and let us know if you see the error?

cohortSql.txt

To answer your question about new tables: you can look at the output of /WebAPI/ddl/results and see if there are any table definitions there that do not match your results schema.

PRijnbeek commented 6 years ago

@chrisknoll i do not see an error when I run that sql directly in DataGrip. Is this the exact same sql run? I remember you mentioned earlier that there is some extra sql added to the SQL that is available in the export tab correct? The error is get is from org.springframework.jdbc i do not think this is also used in DataGrip so not sure if this rules out SQL code that cannot be process by that jdbc driver..

There are many new tables in that /WebAPI/ddl/results i have now all added. Do I now also need to check if certain tables were changed or is that never the case? How would I actually know i have to do this for a new WebApi release? Maybe this should be on the Wiki or in the release notes?

After adding the results tables that were missing I still have the same error.

Is there a way to debug Atlas and see where and with what part of the query this error is generated. It first runs for a while doing the initial queries and then gives the error.

Thanks

chrisknoll commented 6 years ago

There are additional statistics created during cohort generation. I can produce a full sql for you after we check something else:

There was modifications made to three of the results stats tables, which was described here: https://github.com/OHDSI/WebAPI/releases/tag/v2.5.0

ALTER TABLE @resultsSchema.cohort_inclusion_result ADD mode_id  int NOT NULL DEFAULT 0;
ALTER TABLE @resultsSchema.cohort_inclusion_stats ADD mode_id  int NOT NULL DEFAULT 0;
ALTER TABLE @resultsSchema.cohort_summary_stats ADD mode_id  int NOT NULL DEFAULT 0;

To address your points:

The results schema was never managed by WebAPI, it has always been manual work.
We do put notes about new tables/modified tables in the release notes, the 2.6 release notes are not up to date yet, but the 2.5 notes definitely had them, did see the note about the table modifications in the 2.5 release notes, or were you assuming that they weren't there?

PRijnbeek commented 6 years ago

Thanks @chrisknoll

That change in the results schema did the trick :) since we were moving from 2.4 to 2.6.

I did not see that in the webAPI release notes indeed. I looked at the Atlas release notes (considering it is the main application of which WebAPI is a communication layer, but i also probably had to double check the WebAPI release notes).

However I did search here: https://github.com/OHDSI/WebAPI/wiki/Multiple-datasets-configuration which does not contain much information yet, and I remember there was earlier a wiki page with the result tables that need to be added / modified.

so I also looked here: http://www.ohdsi.org/web/wiki/doku.php?id=documentation:software:webapi:multiple_datasets_configuration but that is i think really old :)

It is quite challenging to keep all things up-to-date fully understand that, maybe we should leave the OHDSI wiki all together and only use the Github Wiki as the main documentation point?

Yes it has always been a manual step. I was thinking about this a bit though, maybe we can think of a simple R package that picks all the SQL files in the current /WebAPI/ddl/results and compares this with a result schema. If it is not identical it could flag it or add the missing tables. checkResultSchema(resultSchemaName, connectionDetails, updateSchemas = true, webApiVersion=2.6.0) something like that. This you could then run over all the database versions. Not sure how you are doing this internally with all the many databases you have so maybe there is a smarter way to do this already.

Thanks for your valuable help Chris!

anthonysena commented 6 years ago

If others run into the same problem, please review the "Migration Notes" that appear under the WebAPI releases as shown for v2.6:

https://github.com/OHDSI/WebAPI/releases/tag/v2.6.0

OHDSI / Atlas