Skip to content

SQL Server Migration 2 - #990

Open
labkey-bpatel wants to merge 38 commits into
release26.3-SNAPSHOTfrom
26.3_fb_sql_server_migration_2
Open

SQL Server Migration 2#990
labkey-bpatel wants to merge 38 commits into
release26.3-SNAPSHOTfrom
26.3_fb_sql_server_migration_2

Conversation

@labkey-bpatel

@labkey-bpatel labkey-bpatel commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Rationale

Part 2 of the SQL Server to PostgreSQL migration, continuing from #955. Where #955 covered SQL syntax portability, this PR addresses collation: SQL Server compares text case- and whitespace-insensitively, so source data carrying variants — animal IDs as 4x0133 and 4X0133, units as mg and MG, lookup values differing only in capitalisation — was being collapsed silently at DISTINCT, GROUP BY, UNION and PIVOT time. PostgreSQL compares exactly. Each affected site is now canonicalised in the query, or normalised once at migration time where that repairs more consumers at once. Results were reconciled query by query against the SQL Server source; one report's value legitimately changes, noted below.

Related Pull Requests

Changes

  • Migration-time normalisation in SNPRC_EHRModule — handlers across the snprc_ehr, studydataset, ehr and ehr_lookups schemas lowercase GUID-bearing columns (the SQL Server driver reports ENTITYID/UNIQUEIDENTIFIER as VARCHAR, so the framework's own lowercasing does not reach them) and canonicalise two case-sensitive natural-key FKs against their lookup tables: labwork_services.Dataset against labwork_types.ServiceType, and ehr_lookups.lookups.set_name against lookup_sets.
  • Case and whitespace canonicalisation across the SNPRC reports where a value participates in a GROUP BY, DISTINCT, UNION dedup, PIVOT value list, or a string filter SQL Server was matching case-insensitively — MhcDataPivot, the Parasitology pivots, AdmitComplaintList, BehaviorAbnormalSummary, animalDemographics, colonyUsageQuery, demographicsMostRecentWeight.
  • colonyUsageQuery — gender pivot keys off Id.Demographics.gender.code rather than the label, which resolves to Male/Female on Postgres where SQL Server returned M/F; and the age-pivot column references were re-cased to match their subquery alias, since PIVOT output names are case-sensitive when referenced as quoted identifiers.
  • MhcDataPivotmodified is aggregated with MAX() rather than grouped on as cast(… as varchar); SQL Server's default datetime-to-varchar drops seconds and milliseconds while Postgres keeps them, which was splitting 2028 rows into 2186. A DateTime format string on the column now governs display.
  • ehr_lookups.lab_test_units override — SNPRC-scoped rather than changed in the shared ehr module, because that query is an FK target (its query.xml declares units as the key field) consumed by ehr's results datasets and by WNPRC_EHR, so uppercasing its values should not reach the other centres.
  • PostgreSQL bootstrap script — HL7_IMPORT_LOG.TID becomes a BIGINT identity (PostgreSQL identity columns cannot be NUMERIC), and a missing statement terminator in the is_numeric function is corrected.
  • Java — AnimalsHierarchyController filters visible with a boolean rather than 1; the two clinical-history data sources read timestamps via getTimestamp() instead of parsing the driver's string rendering; SNPRC_schedulerContainerListener runs its container cleanup in a transaction.
  • One report value changes: ReportTcruziSummaryAll. Its inner UNION deduplicated on the test name, and labwork_panels holds TestId 850 as T. CRUZI AB and TestId 965 as T. Cruzi AB — two different test codes distinguishable only by capitalisation. SQL Server treated them as one row and undercounted; Postgres kept both. The query now keys on TestId, which gives the same answer under either collation. Animal 28215 goes from 5 to 6 and the report total from 257 to 258; Postgres's existing values are unchanged. This assumes 850 and 965 are genuinely different assays rather than one test that was re-coded — still an open question for SNPRC.
  • Tests — shared SNPRC EHR setup extracted into SnprcSetupHelper, SNPRC_EHRTest and SNPRC_schedulerTest updated for PostgreSQL, and sample-data Status values normalised.

Tasks 📍

  • Test Automation - fix failures 📌 @labkey-bpatel
  • Code Review
  • Code Review changes
  • TC verify and merge

labkey-bpatel and others added 30 commits May 6, 2026 22:42
… and fix upgrade script. Fix query errors during Folder import.
…ries portable to Postgres (also fixes test failures)
SNPRC_schedulerTest now enables SNPRC_EHR and routes through the helper for SND/EHR domain init, SND category and package seeding, animal group tables, and the Core Facilities/Genetics subfolder plus assay design uploads; moves populateEHRTables() before importStudy() to match SNPRC_EHRTest's populateHardTableRecords-before-import ordering; and turns on import-time query validation.

Guard BeginPage.beginAt against a missing modal-backdrop element so it stops printing a spurious NoSuchElementException stack trace on every navigation.
…UIDs

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The existing snprc_ehr schema handler was leaving GUID FK columns on study datasets (e.g. assay_labworkResults.serviceTestId) uppercase after migration, breaking their FKs to lowercase-canonicalized entityid targets like labwork_panels.ObjectId on Postgres. Extend registerMigrationHandlers with schema contributors for studydataset, ehr, and ehr_lookups so GuidMapperColumn covers those tables too.

Also add LookupSetNameCaseMapperColumn for ehr_lookups.lookups: SS's case-insensitive collation had let set_name values drift out of case with lookup_sets.setname (camelCase 'accessionCode' vs PascalCase 'AccessionCode'), which broke the framework-generated filter queries on Postgres. COALESCE-JOIN against lookup_sets at migration time normalizes to the canonical case.

Dedup key switched from table.getSelectName() to composite '<schema>.<table>' so bare-name collisions across the new schemas can't silently skip a table.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
DISTINCT + GROUP BY on the free-text admitcomplaint column returned different counts on Postgres than SQL Server because SS's case-insensitive collation collapsed variants like 'Trauma' vs 'trauma' or trailing-whitespace copies into single groups while PG kept them apart, dropping some groups below the HAVING count > 10 threshold. Wrap admitcomplaint with UPPER(LTRIM(RTRIM(...))) in both SELECT and GROUP BY so both databases return the same set (45 rows).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Source has case-variant animal IDs (e.g. 4x0133 vs 4X0133) that SS's case-insensitive collation grouped together in the outer GROUP BY; Postgres is case- and whitespace-sensitive so they split into separate output rows, producing 4512 rows on PG vs 4511 on SS. Wrap bn.Id with UPPER(LTRIM(RTRIM(...))) in the SELECT, GROUP BY, and ORDER BY so both databases return the same 4511 rows. The Behavior and BehaviorDescription columns don't have this variance, so those stay unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two overlapping causes of the +158 row discrepancy PG vs SS. First, case/whitespace variants on Id, Haplotype, Ocid, and DataFileSource were splitting groups that SS's collation collapsed — wrap the four columns with UPPER(LTRIM(RTRIM(...))) in the inner subquery so both databases see the same canonical form at GROUP BY and PIVOT time.

Second, SS's default cast(datetime as varchar) is minute-precision while PG's preserves microseconds, so identical-looking timestamps stayed distinct on PG and split groups. Aggregate b.modified via MAX() in the outer SELECT and remove it from GROUP BY; grouping is no longer sensitive to sub-second drift, and the output still shows the latest modification per pivot row. Add a DateTime formatString on modified in the query metadata for consistent display.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
lt.ServiceId.ServiceName has case/whitespace variants in the source data (e.g. 'Ova & Parasites' vs 'OVA & PARASITES'). SS's case-insensitive collation matched both against the NOT IN list; Postgres is case- and whitespace-sensitive so 2059 variants slipped past the exclusion, giving 2077 rows on PG vs the intended 18. Wrap the column with UPPER(RTRIM(LTRIM(...))) so both databases produce 18.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The report's WHERE clauses filter on b.qualresult, b.serviceTestId.testName, and b.serviceTestid.ServiceId.Dataset. SS's case-insensitive collation matched them regardless of source-data casing; Postgres is case-sensitive and drops rows where the case doesn't line up exactly. Wrap each filter column and the CASE b.qualresult expression with UPPER(LTRIM(RTRIM(...))).

Also wrap b.id (animal ID) in the inner SELECTs so case-variant animal IDs collapse in the outer GROUP BY the same way SS does. Same fix applied to both the study and snprc_ehr copies of this query since they carry the same body.

Correctness against the base data still depends on the migration handler widening that lands the missing assay_labworkResults rows on PG; this commit only fixes the query-side case-sensitivity concerns.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
safeAppendTimeFromDate() and safeAppendDateAndTime() read the column with rs.getString() and parsed it against a fixed "yyyy-MM-dd HH:mm:ss[.f]" pattern. The string form is driver-specific, so that pattern has to track every driver's rendering -- it was recently widened to make fractional seconds optional, and a timestamptz column through pgjdbc would still fail on the trailing offset. Read the value as a Timestamp and format from LocalDateTime instead, which removes the dependency on the driver's string form entirely. Also hoists the output DateTimeFormatter into a constant; it was being reallocated on every rendered row.
The SNPRC source data has case/whitespace variants of the same unit (e.g. 'mg' vs 'MG'). SQL Server's case-insensitive collation collapsed them on DISTINCT; Postgres returns them as separate rows. Canonicalize with UPPER(LTRIM(RTRIM(...))) here rather than in the shared ehr module: lab_test_units is an FK target, not just a display list -- its query.xml declares units as the key field, and it is referenced as <fkTable>lab_test_units</fkTable> by ehr's study/chemistryResults, study/hematologyResults and study/urinalysisResults, and directly by WNPRC_EHR's datasets_metadata.xml. Uppercasing changes the set of key values the lookup offers, so it should not be imposed on the other centers.
…fb_sql_server_migration_2

# Conflicts:
#	snprc_ehr/src/org/labkey/snprc_ehr/SNPRC_EHRModule.java
#	snprc_ehr/src/org/labkey/snprc_ehr/history/DefaultTreatmentOrdersDataSource.java
#	snprc_ehr/src/org/labkey/snprc_ehr/history/LabworkDataSource.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants