University of Wisconsin MSSQL to Oracle CDM conversion#6
University of Wisconsin MSSQL to Oracle CDM conversion#6keithwanta wants to merge 6 commits intoARCH-commons:masterfrom
Conversation
| ( | ||
| select DISTINCT PATIENT_NUM from I2B2FACT where START_DATE > to_date('01-Jan-2010','dd-mon-rrrr') | ||
| ) where ROWNUM<100000000 | ||
| / |
There was a problem hiding this comment.
I'm used to seeing a semicolon after SQL statements. Something tells me that ANSI SQL standards has that as a recommendation, but I don't remember. No reason other than that.
There was a problem hiding this comment.
Fair enough. Suggestions for next time:
- review your diffs to make sure your change comment matches them exactly
- separate unrelated changes into separate commits
There was a problem hiding this comment.
I'd rather not take this change, just because I can't fully test Oracle stuff here - we've been relying on one of our sites to do that. Can you remove it from the pull request?
… only relevant for security addition
…s (or blank if setting current_schema)
|
Dan, thanks for catching that! I added the "grant" line above, and inappropriately did a find and replace all, when the others are referencing the CDM.pcornet_med synonym, not the I2B2METADATA.pcornet_med table. I've pushed the fix in my pull request. |
| to_char(start_Date,'HH:MI'), | ||
| end_Date, | ||
| to_char(end_Date,'HH:MI'), | ||
| '', -- providerid, -- <------------------------------------------------------------------- TODO |
There was a problem hiding this comment.
You're commenting out columns that SCILHS sites are required to have. I understand you might have a local branch that comments these out, but please get rid of them from the pull request.
Jeff,
There were a bunch of nested procedure calls within the concept scheme procedures, that I pulled out. Some of the tables were dependencies for the procedures they were in. See sqltext* variables. There are about a dozen of them. The other changes were pretty minor, but you can take a look and let me know what you think should be merged with master.
I may submit one more pull request later on for my performance tweaks, but this should get others through the major road blocks on Oracle if they run the script for the first time.
Keith