Migrate onprc_ehr scripts to PostgreSQL - #1805
Open
labkey-adam wants to merge 13 commits into
Open
Conversation
Contributor
Author
|
@labkey-martyp not sure how we want to handle procedure audit.ArchiveAuditTables() (onprc_ehr-25.002-25.003.sql). PostgreSQL can't do cross-database transactions, so Gemini migrated this procedure to use different schemas in a single database. Note that I didn't bother migrating onprc_ehr-25.000-25.001.sql or onprc_ehr-25.001-25.002.sql since these were just earlier versions of ArchiveAuditTables() that were overwritten. |
Member
|
This should remove the |
…fb_onprc_ehr_scripts
Restore the missing comma before CURRENT_TIMESTAMP in p_Create_TB_Observationrecords (26.000-26.001). CURRENT_TIMESTAMP is a reserved word, so it cannot serve as a bare column alias, and the SELECT supplied 11 values for 12 target columns. plpgsql syntax-checks statement bodies at CREATE FUNCTION time, so the upgrade script itself failed. Also drop the claim that PostgreSQL converts integer modifiedby to timestamp(0) via epoch math: the target column is INT and no such conversion exists. Test ehr_lookups.divider_types.countAsSeparate as a boolean in NHPRoomsUsage. It is boolean on PostgreSQL rather than bit, so comparing it to 0 raised "operator does not exist: boolean = integer" on every call. NOT (subselect) preserves the three-valued behavior, so a missing divider row still falls through to ELSE 1. Prefix plpgsql variables and parameters with v_ where they shadow a column of a table in scope. plpgsql reports such a name as an ambiguous column reference instead of resolving it to the variable, which the @-prefixed SQL Server originals could never hit. MPA_ClnRemarkAddition selected taskId over studyDataset.c6d178_drug, sp_RptNecropsyTissueDistributionUpdates assigned SET taskid = taskId over studydataset.c6d265_tissuedistributions, and sp_PathologyTissueWeightsProcess compared EndDate against studydataset.c6d174_tissue_samples, which declares enddate. Both functions with an EXCEPTION WHEN OTHERS handler swallowed the error and returned 1, so the failures were silent.
Match the stored 'Alive' literal in p_Create_TB_Observationrecords (26.000-26.001). calculated_status is written as 'Alive' by TriggerScriptHelper.getCalculatedStatusValue, so the lowercase comparison carried over from the case-insensitive SQL Server collation matched nothing and the daily TB TST score process silently created no clinical observations. Cast ehr.project.research to int in p_CenterProjectsHistoricalProcess. The column is boolean on PostgreSQL and bit on SQL Server, and PostgreSQL has no assignment cast from boolean to the smallint target column, so both inserts raised a type error that the EXCEPTION WHEN OTHERS handler swallowed.
The ETL framework only inspects a routine's return value when it exposes a return_status or RETURN_VALUE parameter (StoredProcedureStep.getParametersFromDbMetadata). A PostgreSQL function with no arguments exposes neither, so procReturns stayed NONE and the RETURN 1 was discarded — combined with the blanket EXCEPTION WHEN OTHERS handler, a failed run reported success. SQL Server surfaces @RETURN_VALUE through JDBC metadata, so the same failure already fails the step there. Dropping the handler lets the error propagate: doWork wraps it in a PipelineJobException with the original as cause, so the step fails and the log carries the full PostgreSQL error including the CONTEXT line naming the failing statement. The per-table handler in audit.ArchiveAuditTables stays — it rolls back one table's subtransaction, records the failure in ArchiveAuditLog, and deliberately continues to the next table.
Contributor
|
Dropped the two |
PrimaSlideBillingReport and PrimaBlockBillingReport read Prima_slideevents, Prima_LabstationTypes, Prima_slidebases, Prima_userpersons and friends — tables created and then dropped over the SQL Server history, so the PostgreSQL script never creates them. plpgsql doesn't resolve table names at CREATE FUNCTION time, so the upgrade would have succeeded and the functions would have failed only when called. Nothing in the module calls either one. The equivalent SQL Server procedures are broken the same way and should be deleted separately.
Every direct string comparison against a data column now folds both sides with lower(), and the compliance requirement-name matches use ILIKE. These relied on SQL Server's case-insensitive default collation, so a literal port silently matches nothing wherever the stored casing differs — which would have quietly stopped ExpiredProtocolUpdate from expiring protocols and left the compliance rename a no-op. Literals are wrapped rather than pre-lowercased so they stay greppable against the JavaScript and query files that write the same values, and PostgreSQL constant-folds lower('Literal') so there is no runtime cost. None of the affected columns are indexed — LabKey provisions dataset tables with indexes only on container, participantid, date, qcstate, sequencenum and lsid — so no index seek is lost.
core.Principals.Type is left alone: it is a platform-controlled enum only ever written as 'u' or 'g' by server code.
LabKey matches stored procedure parameters by name, so p_RetentionMonths never received the ETL's configured value and the retention ramp collapsed to a 12-month cutoff on the first run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale
We want the onprc_ehr module to run on PostgreSQL
Changes
Notes
lower()calls added to the string comparisons have negligible performance impact, since none of the affected columns are indexed and PostgreSQL constant-folds the literal side.Prima*BillingReportfunctions are not created since thePrima_*tables they read are never created on PostgreSQL (these are created then dropped in the SQL Server scripts); the SQL Server equivalents are broken the same way and should be deleted in a follow-up PR.Tasks 📍