OHDSI / PheValuator

An R package for evaluating phenotype algorithms.
https://ohdsi.github.io/PheValuator/
17 stars 6 forks source link

SQL dialect issue - Postgresql not supported? #11

Closed SSMK-wq closed 4 years ago

SSMK-wq commented 4 years ago

Hi,

I am using PheValuator package and investigating the errors that I see in errorReport.txt.

I was able to see that there are some dialect issue when I tried to execute the error report query in Postgresql. Does PheValuator doesn't support Postgresql?

For ex under the getPopnPrev.sql file, I was able to see the below line

and ((startYear between year('@startDate') and year('@endDate')) or (endYear between year( '@startDate') and year('@endDate'))))

But for the query to work in Postgresql, I had to fix this as shown below (Note I have used DATE keyword)

and ((startYear between year(DATE '@startDate') and year(DATE '@endDate')) or (endYear between year(DATE '@startDate') and year(DATE '@endDate'))))

But good thing is being able to see the errorReport and the query which causes the error. So it helps us to locate and fix the error. In a way useful for us

schuemie commented 4 years ago

@jswerdel : it is recommended to always use explicit casts for literals. In this case I'd write this as

AND ((startYear BETWEEN YEAR(CAST('@startDate' AS DATE)) AND YEAR(CAST('@endDate' AS DATE)) 
OR (endYear BETWEEN YEAR(CAST( '@startDate' AS DATE)) AND YEAR(CAST('@endDate' AS DATE)))))

Furthermore, please adhere to the SQL style guidelines, and make sure you finish your SQL statement with a semicolumn.

Oh, and I wouldn't trust SQL's BETWEEN statement to have the same implementation across platforms. Just use <>= instead to avoid uncertainty about whether BETWEEN is inclusive or exclusive of the start and end values.

jswerdel commented 4 years ago

The changes have been made for postgres sql compatibility.