From 2e9509920362e751b48a086ab5077104e7f3c1fd Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 5 Oct 2026 15:39:31 -0700 Subject: [PATCH 1/5] Highlight missing PostgreSQL upgrade scripts --- .../core/admin/sql/SqlScriptController.java | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/core/src/org/labkey/core/admin/sql/SqlScriptController.java b/core/src/org/labkey/core/admin/sql/SqlScriptController.java index 24cc276833f..a186fdce1ff 100644 --- a/core/src/org/labkey/core/admin/sql/SqlScriptController.java +++ b/core/src/org/labkey/core/admin/sql/SqlScriptController.java @@ -17,6 +17,12 @@ package org.labkey.core.admin.sql; import jakarta.servlet.http.HttpSession; +import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.filefilter.FileFilterUtils; +import org.apache.commons.io.filefilter.IOFileFilter; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -52,12 +58,14 @@ import org.labkey.api.module.Module; import org.labkey.api.module.ModuleContext; import org.labkey.api.module.ModuleLoader; +import org.labkey.api.module.SupportedDatabase; import org.labkey.api.security.AdminConsoleAction; import org.labkey.api.security.Crypt; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.User; import org.labkey.api.security.permissions.AbstractActionPermissionTest; import org.labkey.api.security.permissions.AdminOperationsPermission; +import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.TroubleshooterPermission; import org.labkey.api.settings.AppProps; import org.labkey.api.util.ButtonBuilder; @@ -247,6 +255,7 @@ public ModelAndView getView(ScriptsForm form, BindException errors) throws Excep html.append(LinkBuilder.labkeyLink("consolidate scripts", new ActionURL(ConsolidateScriptsAction.class, ContainerManager.getRoot()))); html.append(LinkBuilder.labkeyLink("orphaned scripts", new ActionURL(OrphanedScriptsAction.class, ContainerManager.getRoot()))); html.append(LinkBuilder.labkeyLink("scripts with errors", new ActionURL(ScriptsWithErrorsAction.class, ContainerManager.getRoot()))); + html.append(LinkBuilder.labkeyLink("missing postgresql scripts", new ActionURL(MissingPostgreSqlScriptsAction.class, ContainerManager.getRoot()))); // html.append(PageFlowUtil.textLink("reorder all scripts", new ActionURL(ReorderAllScriptsAction.class, ContainerManager.getRoot()))); } @@ -1357,6 +1366,15 @@ private static String getTheOtherScriptDir(SqlDialect dialect) Note that `ENTITYID`, `UNIQUEIDENTIFIER`, and `USERID` data types are available on both databases. Maintain these data types when migrating the script (do not replace `ENTITYID` with `VARCHAR(36)` or `USERID` with `INT`, for example). + + Note that the `core.fn_dropifexists` stored procedure is used to drop a TABLE, VIEW, COLUMN, or other database + object if it exists. In most cases, the first parameter specifies the table name, the second parameter specifies + the schema name, the third parameter specifies the object type, and the optional fourth parameter specifies + other details such as a column name. Here are some examples: + - `EXEC core.fn_dropifexists @objname = 'MyTable', @objschema = 'MySchema', @objtype = 'TABLE'` is the same as `DROP TABLE IF EXISTS MySchema.MyTable` + - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'TABLE'` is the same as `DROP TABLE IF EXISTS MySchema.MyTable` + - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'COLUMN', 'MyColumn` is the same as `ALTER TABLE TableName DROP COLUMN IF EXISTS ColumnName` + Convert all core.fn_dropifexists calls to the corresponding native SQL statement, like the three examples above. Include a summary of the changes you made at the end. """; @@ -1439,6 +1457,72 @@ public URLHelper getSuccessURL(SaveScriptForm saveScriptForm) } } + @RequiresPermission(AdminPermission.class) + public class MissingPostgreSqlScriptsAction extends SimpleViewAction + { + @Override + public ModelAndView getView(Object o, BindException errors) + { + HtmlStringBuilder html = HtmlStringBuilder.of(); + ModuleLoader.getInstance().getModules().stream() + .filter(m -> m.getSupportedDatabasesSet().contains(SupportedDatabase.mssql)) + .filter(m -> !StringUtils.isBlank(m.getSourcePath())) + .filter(m -> Strings.CI.contains(m.getName(), "ehr")) + .forEach(m -> { + File ss = new File(m.getSourcePath(), "resources/schemas/dbscripts/sqlserver"); + File pg = new File(m.getSourcePath(), "resources/schemas/dbscripts/postgresql"); + + if (ss.exists() && pg.exists()) + { + Collection diff = CollectionUtils.subtract(listIncrementalScriptNames(ss), listIncrementalScriptNames(pg)); + if (!diff.isEmpty()) + { + html.append(diff.toString()).append(HtmlString.unsafe("
\n")); + } + } + }); + + return new HtmlView(html.isEmpty() ? HtmlString.of("None") : html); + } + + private static final Pattern SCRIPT_PATTERN = Pattern.compile("(\\w+\\.)?\\w+-[0-9]{1,2}\\.[0-9]{2,3}-[0-9]{1,2}\\.[0-9]{2,3}(-\\w+)?.(sql|jsp)"); + + private static final Set SCRIPTS_TO_IGNORE = Set.of( + "ehr-26.001-26.002.sql", // This fixed a SQL Server-specific issue, switching ehr.Project.Created and ehr.Project.Modified to NOT NULL + "onprc_ehr-25.000-25.001.sql", "onprc_ehr-25.001-25.002.sql", // These scripts created early versions of the audit.ArchiveAuditTables proc that were subsequently replaced + "onprc_ehr-26.004-26.005.sql" // onprc_ehr-26.005-26.006.sql is the PG equivalent of this script + ); + + private Collection listIncrementalScriptNames(File dir) + { + return FileUtils.listFiles(dir, new IOFileFilter() + { + @Override + public boolean accept(File file) + { + // We care only about incremental scripts + String name = file.getName(); + return !name.contains("0.000") && SCRIPT_PATTERN.matcher(name).matches() && !SCRIPTS_TO_IGNORE.contains(name); + } + + @Override + public boolean accept(File dir, String name) + { + return false; + } + }, FileFilterUtils.trueFileFilter()).stream() + .map(File::getName) + .toList(); + } + + @Override + public void addNavTrail(NavTree root) + { + new ScriptsAction().addNavTrail(root); + root.addChild("Missing PostgreSQL Incremental Scripts in EHR Modules That Support SQL Server"); + } + } + @RequiresPermission(AdminOperationsPermission.class) public class UnreachableScriptsAction extends SimpleViewAction { From 05fdd9f8c85b08a50b2ea41b980028076037dc96 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 5 Oct 2026 15:59:01 -0700 Subject: [PATCH 2/5] Convert core.fn_dropifexists calls guidance --- core/src/org/labkey/core/admin/sql/SqlScriptController.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/org/labkey/core/admin/sql/SqlScriptController.java b/core/src/org/labkey/core/admin/sql/SqlScriptController.java index a186fdce1ff..72c10737b80 100644 --- a/core/src/org/labkey/core/admin/sql/SqlScriptController.java +++ b/core/src/org/labkey/core/admin/sql/SqlScriptController.java @@ -1324,6 +1324,7 @@ protected ActionURL getSaveScriptActionURL(SqlScript script, String newContents, - Remove unnecessary DROP TABLE statements and core.fn_dropifexists calls, for example, those that come before a table has been created. - Remove all intermediate DROP and ALTER statements that are superseded by later logic. - Remove CREATE TABLE and ALTER TABLE statements followed by DROP TABLE or a core.fn_dropifexists 'TABLE' call on that same table. + - Convert any remaining calls to core.fn_dropifexists into standard DROP IF EXISTS SQL syntax. Include a summary of the changes you made at the end. """; From d02f37926c8b4fd88cdf678844ec46ef8e15a5e6 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 5 Oct 2026 16:24:35 -0700 Subject: [PATCH 3/5] Claude feedback --- .../core/admin/sql/SqlScriptController.java | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/core/src/org/labkey/core/admin/sql/SqlScriptController.java b/core/src/org/labkey/core/admin/sql/SqlScriptController.java index 72c10737b80..256761dc70e 100644 --- a/core/src/org/labkey/core/admin/sql/SqlScriptController.java +++ b/core/src/org/labkey/core/admin/sql/SqlScriptController.java @@ -65,7 +65,6 @@ import org.labkey.api.security.User; import org.labkey.api.security.permissions.AbstractActionPermissionTest; import org.labkey.api.security.permissions.AdminOperationsPermission; -import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.TroubleshooterPermission; import org.labkey.api.settings.AppProps; import org.labkey.api.util.ButtonBuilder; @@ -1317,7 +1316,7 @@ protected ActionURL getSaveScriptActionURL(SqlScript script, String newContents, other details such as a column name. Here are some examples: - `EXEC core.fn_dropifexists @objname = 'MyTable', @objschema = 'MySchema', @objtype = 'TABLE'` is the same as `DROP TABLE IF EXISTS MySchema.MyTable` - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'TABLE'` is the same as `DROP TABLE IF EXISTS MySchema.MyTable` - - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'COLUMN', 'MyColumn` is the same as `ALTER TABLE TableName DROP COLUMN IF EXISTS ColumnName` + - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'COLUMN', 'MyColumn'` is the same as `ALTER TABLE MySchema.MyTable DROP COLUMN IF EXISTS MyColumn` Please do the following: - Consolidate all iterative changes (column additions & renames, PK changes, and FK changes) into the initial CREATE TABLE statements. @@ -1374,7 +1373,7 @@ these data types when migrating the script (do not replace `ENTITYID` with `VARC other details such as a column name. Here are some examples: - `EXEC core.fn_dropifexists @objname = 'MyTable', @objschema = 'MySchema', @objtype = 'TABLE'` is the same as `DROP TABLE IF EXISTS MySchema.MyTable` - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'TABLE'` is the same as `DROP TABLE IF EXISTS MySchema.MyTable` - - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'COLUMN', 'MyColumn` is the same as `ALTER TABLE TableName DROP COLUMN IF EXISTS ColumnName` + - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'COLUMN', 'MyColumn'` is the same as `ALTER TABLE MySchema.MyTable DROP COLUMN IF EXISTS MyColumn` Convert all core.fn_dropifexists calls to the corresponding native SQL statement, like the three examples above. Include a summary of the changes you made at the end. @@ -1458,7 +1457,7 @@ public URLHelper getSuccessURL(SaveScriptForm saveScriptForm) } } - @RequiresPermission(AdminPermission.class) + @RequiresPermission(AdminOperationsPermission.class) public class MissingPostgreSqlScriptsAction extends SimpleViewAction { @Override @@ -1468,14 +1467,15 @@ public ModelAndView getView(Object o, BindException errors) ModuleLoader.getInstance().getModules().stream() .filter(m -> m.getSupportedDatabasesSet().contains(SupportedDatabase.mssql)) .filter(m -> !StringUtils.isBlank(m.getSourcePath())) - .filter(m -> Strings.CI.contains(m.getName(), "ehr")) + .filter(m -> Strings.CI.contains(getRepositoryName(m), "ehr")) .forEach(m -> { File ss = new File(m.getSourcePath(), "resources/schemas/dbscripts/sqlserver"); File pg = new File(m.getSourcePath(), "resources/schemas/dbscripts/postgresql"); - if (ss.exists() && pg.exists()) + if (ss.exists()) { - Collection diff = CollectionUtils.subtract(listIncrementalScriptNames(ss), listIncrementalScriptNames(pg)); + Collection pgScripts = pg.exists() ? listIncrementalScriptNames(pg) : List.of(); + Collection diff = CollectionUtils.subtract(listIncrementalScriptNames(ss), pgScripts); if (!diff.isEmpty()) { html.append(diff.toString()).append(HtmlString.unsafe("
\n")); @@ -1486,7 +1486,8 @@ public ModelAndView getView(Object o, BindException errors) return new HtmlView(html.isEmpty() ? HtmlString.of("None") : html); } - private static final Pattern SCRIPT_PATTERN = Pattern.compile("(\\w+\\.)?\\w+-[0-9]{1,2}\\.[0-9]{2,3}-[0-9]{1,2}\\.[0-9]{2,3}(-\\w+)?.(sql|jsp)"); + // Lookahead rejects bootstrap scripts (from-version 0.00 or 0.000) + private static final Pattern INCREMENTAL_SCRIPT_PATTERN = Pattern.compile("(\\w+\\.)?\\w+-(?!0\\.0{2,3}-)[0-9]{1,2}\\.[0-9]{2,3}-[0-9]{1,2}\\.[0-9]{2,3}(-\\w+)?\\.(sql|jsp)"); private static final Set SCRIPTS_TO_IGNORE = Set.of( "ehr-26.001-26.002.sql", // This fixed a SQL Server-specific issue, switching ehr.Project.Created and ehr.Project.Modified to NOT NULL @@ -1494,6 +1495,18 @@ public ModelAndView getView(Object o, BindException errors) "onprc_ehr-26.004-26.005.sql" // onprc_ehr-26.005-26.006.sql is the PG equivalent of this script ); + // VCS URL is "Unknown" in local builds, so find the enclosing git checkout instead + private static @Nullable String getRepositoryName(Module module) + { + for (File dir = new File(module.getSourcePath()); dir != null; dir = dir.getParentFile()) + { + if (new File(dir, ".git").exists()) + return dir.getName(); + } + + return null; + } + private Collection listIncrementalScriptNames(File dir) { return FileUtils.listFiles(dir, new IOFileFilter() @@ -1501,9 +1514,8 @@ private Collection listIncrementalScriptNames(File dir) @Override public boolean accept(File file) { - // We care only about incremental scripts String name = file.getName(); - return !name.contains("0.000") && SCRIPT_PATTERN.matcher(name).matches() && !SCRIPTS_TO_IGNORE.contains(name); + return INCREMENTAL_SCRIPT_PATTERN.matcher(name).matches() && !SCRIPTS_TO_IGNORE.contains(name); } @Override @@ -1726,8 +1738,9 @@ public void testActionPermissions() assertForAdminOperationsPermission(user, controller.new ConsolidateSchemaAction(), controller.new ConsolidateScriptsAction(), + controller.new MissingPostgreSqlScriptsAction(), controller.new OrphanedScriptsAction(), - new ReorderAllScriptsAction(), + new ReorderAllScriptsAction(), controller.new ReorderScriptAction(), controller.new SaveReorderedScriptAction(), controller.new ScriptAction(), From ee23d5f7d0719c3a9dbe837d5b27e516534c5d6e Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 5 Oct 2026 16:34:13 -0700 Subject: [PATCH 4/5] More Claude feedback --- core/src/org/labkey/core/admin/sql/SqlScriptController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/org/labkey/core/admin/sql/SqlScriptController.java b/core/src/org/labkey/core/admin/sql/SqlScriptController.java index 256761dc70e..89324a006e1 100644 --- a/core/src/org/labkey/core/admin/sql/SqlScriptController.java +++ b/core/src/org/labkey/core/admin/sql/SqlScriptController.java @@ -1475,7 +1475,7 @@ public ModelAndView getView(Object o, BindException errors) if (ss.exists()) { Collection pgScripts = pg.exists() ? listIncrementalScriptNames(pg) : List.of(); - Collection diff = CollectionUtils.subtract(listIncrementalScriptNames(ss), pgScripts); + List diff = CollectionUtils.subtract(listIncrementalScriptNames(ss), pgScripts).stream().sorted().toList(); if (!diff.isEmpty()) { html.append(diff.toString()).append(HtmlString.unsafe("
\n")); From 26f8f950892059a02d8981cbd33b97ddd6439008 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 5 Oct 2026 19:47:38 -0700 Subject: [PATCH 5/5] Ignore some more scripts --- .../org/labkey/core/admin/sql/SqlScriptController.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/org/labkey/core/admin/sql/SqlScriptController.java b/core/src/org/labkey/core/admin/sql/SqlScriptController.java index 89324a006e1..08dcf57963f 100644 --- a/core/src/org/labkey/core/admin/sql/SqlScriptController.java +++ b/core/src/org/labkey/core/admin/sql/SqlScriptController.java @@ -1491,8 +1491,12 @@ public ModelAndView getView(Object o, BindException errors) private static final Set SCRIPTS_TO_IGNORE = Set.of( "ehr-26.001-26.002.sql", // This fixed a SQL Server-specific issue, switching ehr.Project.Created and ehr.Project.Modified to NOT NULL + "extscheduler-26.000-26.001.sql", // extscheduler-26.001-26.002.sql is the PG equivalent of this script "onprc_ehr-25.000-25.001.sql", "onprc_ehr-25.001-25.002.sql", // These scripts created early versions of the audit.ArchiveAuditTables proc that were subsequently replaced - "onprc_ehr-26.004-26.005.sql" // onprc_ehr-26.005-26.006.sql is the PG equivalent of this script + "onprc_ehr-26.004-26.005.sql", // onprc_ehr-26.005-26.006.sql is the PG equivalent of this script + "snprc_ehr-26.000-26.001.sql", // Not needed on PostgreSQL + "tnprc_ehr-26.001-26.002.sql", // Not needed on PostgreSQL + "tnprc_ehr-26.002-26.003.sql" // Not needed on PostgreSQL ); // VCS URL is "Unknown" in local builds, so find the enclosing git checkout instead @@ -1500,7 +1504,7 @@ public ModelAndView getView(Object o, BindException errors) { for (File dir = new File(module.getSourcePath()); dir != null; dir = dir.getParentFile()) { - if (new File(dir, ".git").exists()) + if (FileUtil.appendName(dir, ".git").exists()) return dir.getName(); }