-
Notifications
You must be signed in to change notification settings - Fork 176
Translated postgresql migration scripts to SQL Server up to v2.10 #2238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add sql server migration scripts
src/main/resources/db/migration/sqlserver/V2.9.0.20210727101117__achilles_cache.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/sqlserver/V2.9.0.20210423125133__assets_tags.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/sqlserver/V2.9.0.20210513111520__versioning.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/sqlserver/V2.9.0.20210423125133__assets_tags.sql
Outdated
Show resolved
Hide resolved
Ok, can you confirm that these latest changes work in your environment, and i will proceed with merging the PR. Side note: in the future (not for this PR) you may want to consider creating a branch off of your fork's master branch. I'm not sure what it will look like when we merge this commit into OHDSI/WebAPI master branch and how that will impact your own local fork's master branch. If you work off of branches, then there's no discrepancy between the commits that will be merged into OHDSI master vs. your fork's master (which will come from syncing with upstream). |
Well, I'm experiencing some other issues but it does not seem related. microsoft/OHDSIonAzure#111 |
Ok, one thing we found was that Hibernate is yielding a query that is OK on PG, but not OK on MSSQL: this code is defining a select/count/group by that is not doing the correct group-by clause. On one hand, we only support PG, on the other, I think it's important to yield valid SQL (PG may break later). So I think we should find a fix for this, but I don't think we're going to back-fix this back to the 2.10 release (since we're up to 2.13 now) |
I created an issue that will address this, but it's targeted for v2.14 (it's not exactly a hotfix, we're changing a method type, but in a way that shouldn't incur backwards-breaking change. If you are stuck on v2.10, did you want to try to make a local change on your local env to try to work around this? Note: this is a PG vs. MSSql issue which is why we decided to limit ourselves to 1 platform. Another way to work around this is host WebAPI on PG while you can still host your patient-level-data on your Azure platform...although I think the context you are working on is an all-microsoft solution (the microsoft/ohdsiOnAzure repo) but maybe the pure microsoft solution can still include a PG component? |
Ok, this commit was applied to fix the group-by problem. If you want to copy these changes into your own local env or cherry pick it into a custom branch for your own build purposes, I think that will get you around this issue. |
No description provided.