Use of "+" not compliant

ohdsi-studies / Covid19SubjectsAesiIncidenceRate

Extending on our previous work by Li et al. in understanding the incidence rates of adverse events of special interest (AESI) for COVID-19, this work will look at the rates of these AESIs in patients who had COVDI-19 disease.

https://ohdsi-studies.github.io/Covid19SubjectsAesiIncidenceRate/Protocol.html

2 stars 4 forks source link

Use of "+" not compliant #3

Closed ericaVoss closed 2 years ago

ericaVoss commented 2 years ago

https://github.com/ohdsi-studies/Covid19SubjectsAesiIncidenceRate/blob/master/inst/sql/sql_server/subgroup/S1AgeGroupByGender.sql#L42

There is an errorwith using the “+” operator for string concatenation. BigQuery uses concat(……). SQLRender expects concat() (see https://ohdsi.github.io/SqlRender/articles/UsingSqlRender.html) so the distributed code is not conformant to SQLRender.

schuemie commented 2 years ago

As described in the SqlRender documentation, it is preferred to use the CONCAT() function when concatenating strings. However, SqlRender should have been able to handle this situation, as there are ample cues the values that are being 'summed' are strings. The rules for this were simply missing for BigQuery (but are present for other SQL dialects).

I've added some rules to the BigQuery translation that corrects this. This currently lives in the develop branch of SqlRender. Let me know if you want to use it. (You'd need to modify the renv.lock file).

gowthamrao commented 2 years ago

@ericaVoss and @schuemie wouldnt it be simpler to just change the one line in the SQL? vs relying on develop branch of SqlRender?

ericaVoss commented 2 years ago

I agree with @gowthamrao , using CONCAT() is a simpler update for me to make.

But I +1 the BigQuery update, Michael Kahn will be happy. :)