From 5ae6cd343da8d19ec6774da21c6b892b72a68d37 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 27 Oct 2025 16:54:01 -0700 Subject: [PATCH 1/4] Adjust fetch size. Purge exp.* rows. --- .../labkey/api/data/SqlExecutingSelector.java | 37 +++++++--- api/src/org/labkey/api/data/SqlFactory.java | 5 -- .../data/dialect/BasePostgreSqlDialect.java | 2 +- .../DataClassMigrationSchemaHandler.java | 46 +++++++++++- .../SampleTypeMigrationSchemaHandler.java | 74 +++++++++++++++++-- 5 files changed, 141 insertions(+), 23 deletions(-) diff --git a/api/src/org/labkey/api/data/SqlExecutingSelector.java b/api/src/org/labkey/api/data/SqlExecutingSelector.java index 35429e2794d..aeadad8bf99 100644 --- a/api/src/org/labkey/api/data/SqlExecutingSelector.java +++ b/api/src/org/labkey/api/data/SqlExecutingSelector.java @@ -16,7 +16,6 @@ package org.labkey.api.data; -import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -25,6 +24,7 @@ import org.labkey.api.data.dialect.StatementWrapper; import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.MemTracker; +import org.labkey.api.util.logging.LogHelper; import org.springframework.dao.ConcurrencyFailureException; import org.springframework.jdbc.BadSqlGrammarException; @@ -40,19 +40,21 @@ import static org.labkey.api.util.ExceptionUtil.CALCULATED_COLUMN_SQL_TAG; /** - * Selector that is driven by SQL, which subclasses can control how it's interpreted (LabKey SQL, raw DB SQL, etc) + * Selector that is driven by SQL, which subclasses can control how it's interpreted (LabKey SQL, raw DB SQL, etc.) */ public abstract class SqlExecutingSelector> extends BaseSelector { + private static final Logger LOGGER = LogHelper.getLogger(SqlExecutingSelector.class, "Log warnings about SQL exceptions"); + int _maxRows = Table.ALL_ROWS; protected long _offset = Table.NO_OFFSET; @Nullable Map _namedParameters = null; private ConnectionFactory _connectionFactory = super::getConnection; + private Integer _fetchSize = null; // By default, use the standard fetch size - private @Nullable AsyncQueryRequest _asyncRequest = null; + private @Nullable AsyncQueryRequest _asyncRequest = null; private @Nullable StackTraceElement[] _loggingStacktrace = null; private final QueryLogging _queryLogging; - private static final Logger LOGGER = LogManager.getLogger(SqlExecutingSelector.class); // SQL factory used for the duration of a single query execution. This allows reuse of instances, since query-specific // optimizations won't mutate the ExecutingSelector's externally set state. @@ -111,6 +113,16 @@ public SELECTOR setJdbcCaching(boolean cache) return getThis(); } + /** + * Set a ResultSet fetch size that differs from the default value (1,000 rows on PostgreSQL). This is normally a + * fine fetch size, but not when dealing with rows containing large TEXT or BYTEA columns. + */ + public SELECTOR setFetchSize(int fetchSize) + { + _fetchSize = fetchSize; + return getThis(); + } + @Override protected ResultSetFactory getStandardResultSetFactory() { @@ -282,7 +294,7 @@ protected boolean exists(FACTORY factory) } - void setAsyncRequest(@Nullable AsyncQueryRequest asyncRequest) + void setAsyncRequest(@Nullable AsyncQueryRequest asyncRequest) { _asyncRequest = asyncRequest; @@ -291,7 +303,7 @@ void setAsyncRequest(@Nullable AsyncQueryRequest asyncRequest) } @Nullable - private AsyncQueryRequest getAsyncRequest() + private AsyncQueryRequest getAsyncRequest() { return _asyncRequest; } @@ -466,7 +478,7 @@ public T handleResultSet(ResultSetHandler handler) } } - private ResultSet executeQuery(Connection conn, SQLFragment sqlFragment, boolean scrollable, @Nullable AsyncQueryRequest asyncRequest, @Nullable Integer statementMaxRows) throws SQLException + private ResultSet executeQuery(Connection conn, SQLFragment sqlFragment, boolean scrollable, @Nullable AsyncQueryRequest asyncRequest, @Nullable Integer statementMaxRows) throws SQLException { List parameters = sqlFragment.getParams(); String sql = sqlFragment.getSQL(); @@ -475,13 +487,13 @@ private ResultSet executeQuery(Connection conn, SQLFragment sqlFragment, boolean if (null == parameters || parameters.isEmpty()) { Statement stmt = conn.createStatement(scrollable ? ResultSet.TYPE_SCROLL_INSENSITIVE : ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); - initializeStatement(conn, stmt, asyncRequest, statementMaxRows); + initializeStatement(stmt, asyncRequest, statementMaxRows); rs = stmt.executeQuery(sql); } else { PreparedStatement stmt = conn.prepareStatement(sql, scrollable ? ResultSet.TYPE_SCROLL_INSENSITIVE : ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); - initializeStatement(conn, stmt, asyncRequest, statementMaxRows); + initializeStatement(stmt, asyncRequest, statementMaxRows); try (Parameter.ParameterList jdbcParameters = new Parameter.ParameterList()) { @@ -499,7 +511,7 @@ private ResultSet executeQuery(Connection conn, SQLFragment sqlFragment, boolean return rs; } - private void initializeStatement(Connection conn, Statement stmt, @Nullable AsyncQueryRequest asyncRequest, @Nullable Integer statementMaxRows) throws SQLException + private void initializeStatement(Statement stmt, @Nullable AsyncQueryRequest asyncRequest, @Nullable Integer statementMaxRows) throws SQLException { // Don't set max rows if null or special ALL_ROWS value (we're assuming statement.getMaxRows() defaults to 0, though this isn't actually documented...) if (null != statementMaxRows && Table.ALL_ROWS != statementMaxRows) @@ -507,6 +519,11 @@ private void initializeStatement(Connection conn, Statement stmt, @Nullable Asyn stmt.setMaxRows(statementMaxRows == Table.NO_ROWS ? 1 : statementMaxRows); } + if (null != _fetchSize) + { + stmt.setFetchSize(_fetchSize); + } + if (asyncRequest != null) { asyncRequest.setStatement(stmt); diff --git a/api/src/org/labkey/api/data/SqlFactory.java b/api/src/org/labkey/api/data/SqlFactory.java index 17cb38a382f..6ddf9774e1a 100644 --- a/api/src/org/labkey/api/data/SqlFactory.java +++ b/api/src/org/labkey/api/data/SqlFactory.java @@ -20,11 +20,6 @@ import java.sql.ResultSet; import java.sql.SQLException; -/** - * User: adam - * Date: 1/22/12 - * Time: 2:33 PM - */ public interface SqlFactory { /** Returns the SQL to execute. If null, execution is skipped and handler is called with null parameters, allowing diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index 1bab4c0e186..4d297f83581 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -1810,7 +1810,7 @@ public ConnectionFactory getConnectionFactory(boolean useJdbcCaching, DbScope sc private Closer configureToDisableJdbcCaching(ConnectionWrapper connection, DbScope scope) throws SQLException { - assert connection.getAutoCommit(); // We just got a new connection... it better be set to auto commit + assert connection.getAutoCommit(); // We just got a new connection... it better be set to auto commit try { diff --git a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java index ed1f80f9099..be3da16424e 100644 --- a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java @@ -16,8 +16,11 @@ import org.labkey.api.data.SimpleFilter.InClause; import org.labkey.api.data.SimpleFilter.OrClause; import org.labkey.api.data.SimpleFilter.SQLClause; +import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; +import org.labkey.api.data.dialect.SqlDialect; +import org.labkey.api.exp.OntologyManager; import org.labkey.api.query.FieldKey; import org.labkey.api.util.Formats; import org.labkey.api.util.GUID; @@ -82,10 +85,49 @@ public void addDomainDataFilter(OrClause orClause, DomainFilter filter, TableInf @Override public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter) { - // TODO: delete orphaned rows in exp.Data and exp.Object Collection notCopiedLsids = new TableSelector(sourceTable, Collections.singleton("LSID"), notCopiedFilter, null).getCollection(String.class); if (!notCopiedLsids.isEmpty()) - LOG.info(" {} rows not copied", Formats.commaf0.format(notCopiedLsids.size())); + { + LOG.info(" {} rows not copied -- deleting associated rows from exp.Data, exp.Object, etc.", Formats.commaf0.format(notCopiedLsids.size())); + SqlDialect dialect = sourceTable.getSqlDialect(); + SqlExecutor executor = new SqlExecutor(OntologyManager.getExpSchema()); + + // Delete from exp.Data (and associated tables) + executor.execute( + new SQLFragment("DELETE FROM exp.DataInput WHERE DataId IN (SELECT RowId FROM exp.Data WHERE LSID") + .appendInClause(notCopiedLsids, dialect) + .append(")") + ); + executor.execute( + new SQLFragment("DELETE FROM exp.DataAliasMap WHERE LSID") + .appendInClause(notCopiedLsids, dialect) + ); + executor.execute( + new SQLFragment("DELETE FROM exp.Data WHERE LSID") + .appendInClause(notCopiedLsids, dialect) + ); + + // Delete from exp.Object (and associated tables) + executor.execute( + new SQLFragment("DELETE FROM exp.Edge WHERE FromObjectId IN (SELECT ObjectId FROM exp.Object WHERE ObjectURI") + .appendInClause(notCopiedLsids, dialect) + .append(")") + ); + executor.execute( + new SQLFragment("DELETE FROM exp.Edge WHERE ToObjectId IN (SELECT ObjectId FROM exp.Object WHERE ObjectURI") + .appendInClause(notCopiedLsids, dialect) + .append(")") + ); + executor.execute( + new SQLFragment("DELETE FROM exp.ObjectProperty WHERE ObjectId IN (SELECT ObjectId FROM exp.Object WHERE ObjectURI") + .appendInClause(notCopiedLsids, dialect) + .append(")") + ); + executor.execute( + new SQLFragment("DELETE FROM exp.Object WHERE ObjectURI") + .appendInClause(notCopiedLsids, dialect) + ); + } String name = sourceTable.getName(); int idx = name.indexOf('_'); diff --git a/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java index 563b4304431..861006e1ea9 100644 --- a/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java @@ -2,16 +2,20 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; -import org.labkey.api.collections.CsvSet; import org.labkey.api.data.DatabaseMigrationService.DefaultMigrationSchemaHandler; import org.labkey.api.data.DatabaseMigrationService.DomainFilter; import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.Selector; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SimpleFilter.FilterClause; import org.labkey.api.data.SimpleFilter.OrClause; import org.labkey.api.data.SimpleFilter.SQLClause; +import org.labkey.api.data.SqlExecutor; +import org.labkey.api.data.SqlSelector; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; +import org.labkey.api.data.dialect.SqlDialect; +import org.labkey.api.exp.OntologyManager; import org.labkey.api.exp.api.SampleTypeDomainKind; import org.labkey.api.query.FieldKey; import org.labkey.api.util.Formats; @@ -19,6 +23,7 @@ import org.labkey.api.util.logging.LogHelper; import java.util.Collection; +import java.util.Collections; import java.util.Set; class SampleTypeMigrationSchemaHandler extends DefaultMigrationSchemaHandler @@ -88,9 +93,68 @@ private String getJoinColumnName(TableInfo sourceTable) @Override public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter) { - // TODO: delete orphaned rows in exp.Material and exp.Object. Be sure to handle no RowId case. - Collection notCopiedLsids = new TableSelector(sourceTable, new CsvSet("LSID, RowId"), notCopiedFilter, null).getCollection(String.class); - if (!notCopiedLsids.isEmpty()) - LOG.info(" {} rows not copied", Formats.commaf0.format(notCopiedLsids.size())); + SqlDialect dialect = sourceTable.getSqlDialect(); + final Selector selector; + + if (getJoinColumnName(sourceTable).equals("LSID")) + { + // SELECT RowId FROM exp.Material m WHERE lsid IN (SELECT lsid FROM expsampleset.c3258d9570_dinosaurs WHERE ...) + selector = new SqlSelector(getSchema(), new SQLFragment("SELECT RowId FROM exp.Material m WHERE lsid IN (SELECT lsid FROM ") + .appendIdentifier(sourceTable.getSelectName()) + .append(" WHERE ") + .append(notCopiedFilter.getSQLFragment(dialect)) + .append(")") + ); + } + else + { + selector = new TableSelector(sourceTable, Collections.singleton("RowId"), notCopiedFilter, null); + } + + Collection notCopiedRows = selector.getCollection(Integer.class); + + if (!notCopiedRows.isEmpty()) + { + LOG.info(" {} rows not copied -- deleting associated rows from exp.Material, exp.Object, etc.", Formats.commaf0.format(notCopiedRows.size())); + + SqlExecutor executor = new SqlExecutor(OntologyManager.getExpSchema()); + + // An IN clause of exp.Material.RowIds are also the associated ObjectIds + SQLFragment objectIdClause = new SQLFragment() + .appendInClause(notCopiedRows, dialect); + + // Delete from exp.Material (and associated tables) + executor.execute( + new SQLFragment("DELETE FROM exp.MaterialInput WHERE MaterialId") + .append(objectIdClause) + ); + executor.execute( + new SQLFragment("DELETE FROM exp.MaterialAliasMap WHERE LSID IN (SELECT LSID FROM exp.Material WHERE RowId") + .append(objectIdClause) + .append(")") + ); + executor.execute( + new SQLFragment("DELETE FROM exp.Material WHERE RowId") + .append(objectIdClause) + ); + + // Delete from exp.Object (and associated tables) + executor.execute( + new SQLFragment("DELETE FROM exp.Edge WHERE FromObjectId") + .append(objectIdClause) + ); + executor.execute( + new SQLFragment("DELETE FROM exp.Edge WHERE ToObjectId") + .append(objectIdClause) + ); + executor.execute( + new SQLFragment("DELETE FROM exp.ObjectProperty WHERE ObjectId") + .append(objectIdClause) + ); + executor.execute( + new SQLFragment("DELETE FROM exp.Object WHERE ObjectId") + .append(objectIdClause) + ); + } } } From 40babb1b27033931c0ae122a8530e64ff33ef29a Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 29 Oct 2025 11:56:32 -0700 Subject: [PATCH 2/4] Delete orphaned rows from exp.* tables during migration. Optimize exp.Data delete by adding FK indices. --- .../labkey/api/data/AggregateColumnInfo.java | 2 +- api/src/org/labkey/api/data/ForeignKey.java | 2 +- .../labkey/api/util/StringUtilsLabKey.java | 1 - .../postgresql/exp-25.011-25.012.sql | 3 ++ .../dbscripts/sqlserver/exp-25.011-25.012.sql | 3 ++ .../DataClassMigrationSchemaHandler.java | 54 +++++++++++-------- .../ExperimentMigrationSchemaHandler.java | 3 +- .../labkey/experiment/ExperimentModule.java | 2 +- .../SampleTypeMigrationSchemaHandler.java | 16 +++--- .../org/labkey/issue/query/IssuesTable.java | 2 +- .../src/org/labkey/query/sql/QueryPivot.java | 2 +- 11 files changed, 55 insertions(+), 35 deletions(-) create mode 100644 experiment/resources/schemas/dbscripts/postgresql/exp-25.011-25.012.sql create mode 100644 experiment/resources/schemas/dbscripts/sqlserver/exp-25.011-25.012.sql diff --git a/api/src/org/labkey/api/data/AggregateColumnInfo.java b/api/src/org/labkey/api/data/AggregateColumnInfo.java index 0ad08409f99..c5051396488 100644 --- a/api/src/org/labkey/api/data/AggregateColumnInfo.java +++ b/api/src/org/labkey/api/data/AggregateColumnInfo.java @@ -108,7 +108,7 @@ public StringExpression getURL(ColumnInfo parent) } @Override - public NamedObjectList getSelectList(RenderContext ctx) + public @NotNull NamedObjectList getSelectList(RenderContext ctx) { return fk.getSelectList(ctx); } diff --git a/api/src/org/labkey/api/data/ForeignKey.java b/api/src/org/labkey/api/data/ForeignKey.java index baeb862751e..3f3afdb3b5f 100644 --- a/api/src/org/labkey/api/data/ForeignKey.java +++ b/api/src/org/labkey/api/data/ForeignKey.java @@ -32,7 +32,7 @@ /** * Interface describing a ColumnInfo's foreign key relationship, which might be a "real" FK in the underlying - * database of a "soft" FK, making it a lookup to the foreign key's target. + * database or a "soft" FK, making it a lookup to the foreign key's target. */ public interface ForeignKey { diff --git a/api/src/org/labkey/api/util/StringUtilsLabKey.java b/api/src/org/labkey/api/util/StringUtilsLabKey.java index f57f9ece841..4720a80d4d0 100644 --- a/api/src/org/labkey/api/util/StringUtilsLabKey.java +++ b/api/src/org/labkey/api/util/StringUtilsLabKey.java @@ -26,7 +26,6 @@ import org.junit.Test; import org.labkey.api.data.Container; import org.labkey.api.exp.Identifiable; -import org.labkey.api.security.Encryption; import org.labkey.api.view.ViewServlet; import java.nio.charset.Charset; diff --git a/experiment/resources/schemas/dbscripts/postgresql/exp-25.011-25.012.sql b/experiment/resources/schemas/dbscripts/postgresql/exp-25.011-25.012.sql new file mode 100644 index 00000000000..002a7300b48 --- /dev/null +++ b/experiment/resources/schemas/dbscripts/postgresql/exp-25.011-25.012.sql @@ -0,0 +1,3 @@ +-- These tables have FKs to exp.Data without corresponding indices. Add indices to speed up exp.Data delete. +CREATE INDEX IX_DataInput_DataId ON exp.DataInput (DataId); +CREATE INDEX IX_DataAncestors_RowId ON exp.DataAncestors (RowId); diff --git a/experiment/resources/schemas/dbscripts/sqlserver/exp-25.011-25.012.sql b/experiment/resources/schemas/dbscripts/sqlserver/exp-25.011-25.012.sql new file mode 100644 index 00000000000..002a7300b48 --- /dev/null +++ b/experiment/resources/schemas/dbscripts/sqlserver/exp-25.011-25.012.sql @@ -0,0 +1,3 @@ +-- These tables have FKs to exp.Data without corresponding indices. Add indices to speed up exp.Data delete. +CREATE INDEX IX_DataInput_DataId ON exp.DataInput (DataId); +CREATE INDEX IX_DataAncestors_RowId ON exp.DataAncestors (RowId); diff --git a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java index be3da16424e..25cb0e42483 100644 --- a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java @@ -20,7 +20,7 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; import org.labkey.api.data.dialect.SqlDialect; -import org.labkey.api.exp.OntologyManager; +import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.query.FieldKey; import org.labkey.api.util.Formats; import org.labkey.api.util.GUID; @@ -85,47 +85,59 @@ public void addDomainDataFilter(OrClause orClause, DomainFilter filter, TableInf @Override public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter) { + // exp.Data has an index on ObjectId, so use ObjectIds to delete from exp.Data tables as well as exp.Object. + // Our notCopiedFilter works on the data class provisioned table, so we need to select LSIDs from that table + // and then select from exp.Data to map those LSIDs to ObjectIds. Collection notCopiedLsids = new TableSelector(sourceTable, Collections.singleton("LSID"), notCopiedFilter, null).getCollection(String.class); + if (!notCopiedLsids.isEmpty()) { - LOG.info(" {} rows not copied -- deleting associated rows from exp.Data, exp.Object, etc.", Formats.commaf0.format(notCopiedLsids.size())); - SqlDialect dialect = sourceTable.getSqlDialect(); - SqlExecutor executor = new SqlExecutor(OntologyManager.getExpSchema()); + SimpleFilter dataFilter = new SimpleFilter(new InClause(FieldKey.fromParts("LSID"), notCopiedLsids)); + Collection notCopiedObjectIds = new TableSelector(ExperimentService.get().getTinfoData(), Collections.singleton("ObjectId"), dataFilter, null).getCollection(Integer.class); + + LOG.info(" {} rows not copied -- deleting associated rows from exp.Data, exp.Object, etc.", Formats.commaf0.format(notCopiedObjectIds.size())); + SqlExecutor executor = new SqlExecutor(ExperimentService.get().getSchema()); + SqlDialect dialect = ExperimentService.get().getSchema().getSqlDialect(); // Delete from exp.Data (and associated tables) + LOG.info(" exp.DataInput"); executor.execute( - new SQLFragment("DELETE FROM exp.DataInput WHERE DataId IN (SELECT RowId FROM exp.Data WHERE LSID") - .appendInClause(notCopiedLsids, dialect) + new SQLFragment("DELETE FROM exp.DataInput WHERE DataId IN (SELECT RowId FROM exp.Data WHERE ObjectId") + .appendInClause(notCopiedObjectIds, dialect) .append(")") ); + LOG.info(" exp.DataAliasMap"); executor.execute( - new SQLFragment("DELETE FROM exp.DataAliasMap WHERE LSID") - .appendInClause(notCopiedLsids, dialect) + new SQLFragment("DELETE FROM exp.DataAliasMap WHERE LSID IN (SELECT LSID FROM exp.Data WHERE ObjectId") // TODO: Use notCopiedLsids instead? + .appendInClause(notCopiedObjectIds, dialect) + .append(")") ); + LOG.info(" exp.Data"); executor.execute( - new SQLFragment("DELETE FROM exp.Data WHERE LSID") - .appendInClause(notCopiedLsids, dialect) + new SQLFragment("DELETE FROM exp.Data WHERE ObjectId") + .appendInClause(notCopiedObjectIds, dialect) ); // Delete from exp.Object (and associated tables) + LOG.info(" exp.Edge (FromObjectId)"); executor.execute( - new SQLFragment("DELETE FROM exp.Edge WHERE FromObjectId IN (SELECT ObjectId FROM exp.Object WHERE ObjectURI") - .appendInClause(notCopiedLsids, dialect) - .append(")") + new SQLFragment("DELETE FROM exp.Edge WHERE FromObjectId") + .appendInClause(notCopiedObjectIds, dialect) ); + LOG.info(" exp.Edge (ToObjectId)"); executor.execute( - new SQLFragment("DELETE FROM exp.Edge WHERE ToObjectId IN (SELECT ObjectId FROM exp.Object WHERE ObjectURI") - .appendInClause(notCopiedLsids, dialect) - .append(")") + new SQLFragment("DELETE FROM exp.Edge WHERE ToObjectId") + .appendInClause(notCopiedObjectIds, dialect) ); + LOG.info(" exp.ObjectProperty"); executor.execute( - new SQLFragment("DELETE FROM exp.ObjectProperty WHERE ObjectId IN (SELECT ObjectId FROM exp.Object WHERE ObjectURI") - .appendInClause(notCopiedLsids, dialect) - .append(")") + new SQLFragment("DELETE FROM exp.ObjectProperty WHERE ObjectId") + .appendInClause(notCopiedObjectIds, dialect) ); + LOG.info(" exp.Object"); executor.execute( - new SQLFragment("DELETE FROM exp.Object WHERE ObjectURI") - .appendInClause(notCopiedLsids, dialect) + new SQLFragment("DELETE FROM exp.Object WHERE ObjectId") + .appendInClause(notCopiedObjectIds, dialect) ); } diff --git a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java index 24efd1d0426..405f0fe6253 100644 --- a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java @@ -58,9 +58,10 @@ public void beforeSchema() @Override public List getTablesToCopy() { - // No need to populate the MaterialIndexed table -- new server should be completely re-indexed after migration + // No need to populate the MaterialIndexed or DataIndexed tables -- new server should be completely re-indexed after migration List tables = super.getTablesToCopy(); tables.remove(ExperimentServiceImpl.get().getTinfoMaterialIndexed()); + tables.remove(ExperimentServiceImpl.get().getTinfoDataIndexed()); return tables; } diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 7c57a79c011..9e6c3acf646 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -196,7 +196,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 25.011; + return 25.012; } @Nullable diff --git a/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java index 861006e1ea9..b2370a9ac8e 100644 --- a/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java @@ -98,13 +98,8 @@ public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilte if (getJoinColumnName(sourceTable).equals("LSID")) { - // SELECT RowId FROM exp.Material m WHERE lsid IN (SELECT lsid FROM expsampleset.c3258d9570_dinosaurs WHERE ...) - selector = new SqlSelector(getSchema(), new SQLFragment("SELECT RowId FROM exp.Material m WHERE lsid IN (SELECT lsid FROM ") - .appendIdentifier(sourceTable.getSelectName()) - .append(" WHERE ") - .append(notCopiedFilter.getSQLFragment(dialect)) - .append(")") - ); + Collection lsids = new TableSelector(sourceTable, Collections.singleton("LSID"), notCopiedFilter, null).getCollection(String.class); + selector = new SqlSelector(getSchema(), new SQLFragment("SELECT RowId FROM exp.Material m WHERE lsid").appendInClause(lsids, dialect)); } else { @@ -124,33 +119,40 @@ public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilte .appendInClause(notCopiedRows, dialect); // Delete from exp.Material (and associated tables) + LOG.info(" exp.MaterialInput"); executor.execute( new SQLFragment("DELETE FROM exp.MaterialInput WHERE MaterialId") .append(objectIdClause) ); + LOG.info(" exp.MaterialAliasMap"); executor.execute( new SQLFragment("DELETE FROM exp.MaterialAliasMap WHERE LSID IN (SELECT LSID FROM exp.Material WHERE RowId") .append(objectIdClause) .append(")") ); + LOG.info(" exp.Material"); executor.execute( new SQLFragment("DELETE FROM exp.Material WHERE RowId") .append(objectIdClause) ); // Delete from exp.Object (and associated tables) + LOG.info(" exp.Edge (FromObjectId)"); executor.execute( new SQLFragment("DELETE FROM exp.Edge WHERE FromObjectId") .append(objectIdClause) ); + LOG.info(" exp.Edge (ToObjectId)"); executor.execute( new SQLFragment("DELETE FROM exp.Edge WHERE ToObjectId") .append(objectIdClause) ); + LOG.info(" exp.ObjectProperty"); executor.execute( new SQLFragment("DELETE FROM exp.ObjectProperty WHERE ObjectId") .append(objectIdClause) ); + LOG.info(" exp.Object"); executor.execute( new SQLFragment("DELETE FROM exp.Object WHERE ObjectId") .append(objectIdClause) diff --git a/issues/src/org/labkey/issue/query/IssuesTable.java b/issues/src/org/labkey/issue/query/IssuesTable.java index d9e4c6dcf12..f9f526319d1 100644 --- a/issues/src/org/labkey/issue/query/IssuesTable.java +++ b/issues/src/org/labkey/issue/query/IssuesTable.java @@ -796,7 +796,7 @@ public AssignedToForeignKey(UserSchema schema) } @Override - public NamedObjectList getSelectList(RenderContext ctx) + public @NotNull NamedObjectList getSelectList(RenderContext ctx) { NamedObjectList objectList = new NamedObjectList(); Integer issueId = ctx.get(FieldKey.fromParts("IssueId"), Integer.class); diff --git a/query/src/org/labkey/query/sql/QueryPivot.java b/query/src/org/labkey/query/sql/QueryPivot.java index e281c95cbcd..42175beebcc 100644 --- a/query/src/org/labkey/query/sql/QueryPivot.java +++ b/query/src/org/labkey/query/sql/QueryPivot.java @@ -1181,7 +1181,7 @@ public StringExpression getURL(ColumnInfo parent) } @Override - public NamedObjectList getSelectList(RenderContext ctx) + public @NotNull NamedObjectList getSelectList(RenderContext ctx) { return new NamedObjectList(); } From d7a99ea8e374327dc647c08310fb3b68b6353878 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 29 Oct 2025 12:55:34 -0700 Subject: [PATCH 3/4] Comments --- api/src/org/labkey/api/data/ForeignKey.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/data/ForeignKey.java b/api/src/org/labkey/api/data/ForeignKey.java index 3f3afdb3b5f..1b0fa2628ff 100644 --- a/api/src/org/labkey/api/data/ForeignKey.java +++ b/api/src/org/labkey/api/data/ForeignKey.java @@ -37,7 +37,7 @@ public interface ForeignKey { /** - * Return a new lookup column with the specified displayField. If displayField is null, then this method + * Return a new lookup column with the specified displayField. If displayField is null, then this method * should return the title column (default display field) of the foreign table. * It should return null if there is no default display field. * The ColumnInfo parent is a column which has the value of the foreign key. @@ -47,8 +47,8 @@ public interface ForeignKey ColumnInfo createLookupColumn(ColumnInfo parent, String displayField); /** - * Return the TableInfo for the foreign table. This TableInfo can be used to discover the names of available - * columns in a UI. The returned TableInfo will not necessarily be one that can be used for querying (e.g. passing + * Return the TableInfo for the foreign table. This TableInfo can be used to discover the names of available + * columns in a UI. The returned TableInfo will not necessarily be one that can be used for querying (e.g. passing * to Table.select... */ @Nullable @@ -61,7 +61,7 @@ default TableDescription getLookupTableDescription() } /** - * Return an URL expression for what the hyperlink for this column should be. The hyperlink must be able to be + * Return a URL expression for what the hyperlink for this column should be. The hyperlink must be able to be * constructed knowing only the foreign key value, as other columns may not be available in the ResultSet. */ @Nullable StringExpression getURL(ColumnInfo parent); @@ -72,7 +72,7 @@ default TableDescription getLookupTableDescription() @NotNull NamedObjectList getSelectList(RenderContext ctx); /** - * @return The container id of the foreign user schema table. Null means current container. + * @return The container id of the foreign user schema table. Null means current container. */ Container getLookupContainer(); @@ -111,7 +111,7 @@ default String getLookupSchemaName() /** * Fixup any references fo FieldKeys that may have been reparented or renamed by Query and - * generate a new ForeignKey. If fixup is not needed, return null. + * generate a new ForeignKey. If fixup is not needed, return null. * * @param parent A new parent FieldKey to inject, e.g. "title" becomes "parent/title". * @param mapping Rename FieldKeys, e.g. "foo" becomes "bar". From 8138c8c4bfba7faa6a4e57533946b16ee058edf1 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 29 Oct 2025 15:11:28 -0700 Subject: [PATCH 4/4] Add index to speed up exp.Material deletes --- .../org/labkey/api/view/FolderManagement.java | 20 +++++------ .../postgresql/exp-25.012-25.013.sql | 4 +++ .../dbscripts/sqlserver/exp-25.012-25.013.sql | 4 +++ .../DataClassMigrationSchemaHandler.java | 33 ++++--------------- .../ExperimentMigrationSchemaHandler.java | 31 +++++++++++++++++ .../labkey/experiment/ExperimentModule.java | 2 +- .../SampleTypeMigrationSchemaHandler.java | 22 +------------ 7 files changed, 58 insertions(+), 58 deletions(-) create mode 100644 experiment/resources/schemas/dbscripts/postgresql/exp-25.012-25.013.sql create mode 100644 experiment/resources/schemas/dbscripts/sqlserver/exp-25.012-25.013.sql diff --git a/api/src/org/labkey/api/view/FolderManagement.java b/api/src/org/labkey/api/view/FolderManagement.java index fc207c5a013..971a5052f2c 100644 --- a/api/src/org/labkey/api/view/FolderManagement.java +++ b/api/src/org/labkey/api/view/FolderManagement.java @@ -47,7 +47,7 @@ public enum TYPE FolderManagement { @Override - void addNavTrail(BaseViewAction action, NavTree root, Container c, User user) + void addNavTrail(BaseViewAction action, NavTree root, Container c, User user) { // In the root, view is rendered as a standalone page (no tab strip). Create an appropriate nav trail. if (c.isRoot()) @@ -76,7 +76,7 @@ boolean shouldRenderTabStrip(Container container) ProjectSettings // Used for project settings { @Override - void addNavTrail(BaseViewAction action, NavTree root, Container c, User user) + void addNavTrail(BaseViewAction action, NavTree root, Container c, User user) { action.setHelpTopic("customizeLook"); root.addChild("Project Settings"); @@ -91,7 +91,7 @@ boolean shouldRenderTabStrip(Container container) LookAndFeelSettings // Used for the admin console actions -- allows for troubleshooter permissions { @Override - void addNavTrail(BaseViewAction action, NavTree root, Container c, User user) + void addNavTrail(BaseViewAction action, NavTree root, Container c, User user) { action.setHelpTopic("customizeLook"); PageFlowUtil.urlProvider(AdminUrls.class).addAdminNavTrail(root, "Look and Feel Settings", action.getClass(), c); @@ -104,7 +104,7 @@ boolean shouldRenderTabStrip(Container container) } }; - abstract void addNavTrail(BaseViewAction action, NavTree root, Container c, User user); + abstract void addNavTrail(BaseViewAction action, NavTree root, Container c, User user); abstract boolean shouldRenderTabStrip(Container container); } @@ -170,14 +170,14 @@ public List getTabList() protected abstract List getTabProviders(); @Override - public abstract HttpView getTabView(String tabId) throws Exception; + public abstract HttpView getTabView(String tabId) throws Exception; } /** * Marker interface for actions that register themselves with a management page */ - interface ManagementAction extends Controller + public interface ManagementAction extends Controller { } @@ -208,7 +208,7 @@ public final ModelAndView getView(Object form, BindException errors) throws Exce } protected abstract TYPE getType(); - protected abstract HttpView getTabView() throws Exception; + protected abstract HttpView getTabView() throws Exception; @Override public void addNavTrail(NavTree root) @@ -244,7 +244,7 @@ public URLHelper getSuccessURL(FORM form) protected abstract TYPE getType(); - protected abstract HttpView getTabView(FORM form, boolean reshow, BindException errors) throws Exception; + protected abstract HttpView getTabView(FORM form, boolean reshow, BindException errors) throws Exception; @Override public void addNavTrail(NavTree root) @@ -255,7 +255,7 @@ public void addNavTrail(NavTree root) /** Wrap the provided view in a tab strip, if that's what the type desires **/ - private static HttpView wrapViewInTabStrip(BaseViewAction action, TYPE type, HttpView view, BindException errors) + private static HttpView wrapViewInTabStrip(BaseViewAction action, TYPE type, HttpView view, BindException errors) { ViewContext ctx = action.getViewContext(); Container c = ctx.getContainer(); @@ -268,7 +268,7 @@ private static HttpView wrapViewInTabStrip(BaseViewAction action, TYPE type, Htt return new ManagementTabStrip(c, tabId, errors) { @Override - public HttpView getTabView(String tabId) + public HttpView getTabView(String tabId) { return view; } diff --git a/experiment/resources/schemas/dbscripts/postgresql/exp-25.012-25.013.sql b/experiment/resources/schemas/dbscripts/postgresql/exp-25.012-25.013.sql new file mode 100644 index 00000000000..2ec83f837a9 --- /dev/null +++ b/experiment/resources/schemas/dbscripts/postgresql/exp-25.012-25.013.sql @@ -0,0 +1,4 @@ +-- Not needed since it's redundant with exp.DataInput's PK +DROP INDEX exp.IX_DataInput_DataId; + +CREATE INDEX IX_MaterialAncestors_RowId ON exp.MaterialAncestors (RowId); diff --git a/experiment/resources/schemas/dbscripts/sqlserver/exp-25.012-25.013.sql b/experiment/resources/schemas/dbscripts/sqlserver/exp-25.012-25.013.sql new file mode 100644 index 00000000000..fc5c330f5a9 --- /dev/null +++ b/experiment/resources/schemas/dbscripts/sqlserver/exp-25.012-25.013.sql @@ -0,0 +1,4 @@ +-- Not needed since it's redundant with exp.DataInput's PK +DROP INDEX IX_DataInput_DataId ON exp.DataInput; + +CREATE INDEX IX_MaterialAncestors_RowId ON exp.MaterialAncestors (RowId); diff --git a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java index 25cb0e42483..4d0d8a09f12 100644 --- a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java @@ -98,47 +98,28 @@ public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilte LOG.info(" {} rows not copied -- deleting associated rows from exp.Data, exp.Object, etc.", Formats.commaf0.format(notCopiedObjectIds.size())); SqlExecutor executor = new SqlExecutor(ExperimentService.get().getSchema()); SqlDialect dialect = ExperimentService.get().getSchema().getSqlDialect(); + SQLFragment objectIdClause = new SQLFragment() + .appendInClause(notCopiedObjectIds, dialect); // Delete from exp.Data (and associated tables) LOG.info(" exp.DataInput"); executor.execute( new SQLFragment("DELETE FROM exp.DataInput WHERE DataId IN (SELECT RowId FROM exp.Data WHERE ObjectId") - .appendInClause(notCopiedObjectIds, dialect) + .append(objectIdClause) .append(")") ); LOG.info(" exp.DataAliasMap"); executor.execute( - new SQLFragment("DELETE FROM exp.DataAliasMap WHERE LSID IN (SELECT LSID FROM exp.Data WHERE ObjectId") // TODO: Use notCopiedLsids instead? - .appendInClause(notCopiedObjectIds, dialect) - .append(")") + new SQLFragment("DELETE FROM exp.DataAliasMap WHERE LSID") + .appendInClause(notCopiedLsids, dialect) ); LOG.info(" exp.Data"); executor.execute( new SQLFragment("DELETE FROM exp.Data WHERE ObjectId") - .appendInClause(notCopiedObjectIds, dialect) + .append(objectIdClause) ); - // Delete from exp.Object (and associated tables) - LOG.info(" exp.Edge (FromObjectId)"); - executor.execute( - new SQLFragment("DELETE FROM exp.Edge WHERE FromObjectId") - .appendInClause(notCopiedObjectIds, dialect) - ); - LOG.info(" exp.Edge (ToObjectId)"); - executor.execute( - new SQLFragment("DELETE FROM exp.Edge WHERE ToObjectId") - .appendInClause(notCopiedObjectIds, dialect) - ); - LOG.info(" exp.ObjectProperty"); - executor.execute( - new SQLFragment("DELETE FROM exp.ObjectProperty WHERE ObjectId") - .appendInClause(notCopiedObjectIds, dialect) - ); - LOG.info(" exp.Object"); - executor.execute( - new SQLFragment("DELETE FROM exp.Object WHERE ObjectId") - .appendInClause(notCopiedObjectIds, dialect) - ); + ExperimentMigrationSchemaHandler.deleteObjectIds(objectIdClause); } String name = sourceTable.getName(); diff --git a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java index 405f0fe6253..9515e96a490 100644 --- a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java @@ -1,5 +1,6 @@ package org.labkey.experiment; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.CompareType; import org.labkey.api.data.CompareType.CompareClause; @@ -17,6 +18,7 @@ import org.labkey.api.exp.OntologyManager; import org.labkey.api.query.FieldKey; import org.labkey.api.util.GUID; +import org.labkey.api.util.logging.LogHelper; import org.labkey.experiment.api.ExperimentServiceImpl; import java.util.List; @@ -25,6 +27,8 @@ class ExperimentMigrationSchemaHandler extends DefaultMigrationSchemaHandler { + private static final Logger LOG = LogHelper.getLogger(ExperimentMigrationSchemaHandler.class, "Progress of exp deletes during database migration"); + public ExperimentMigrationSchemaHandler() { super(OntologyManager.getExpSchema()); @@ -125,4 +129,31 @@ public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema s new SqlExecutor(getSchema()).execute("ALTER TABLE exp.Object ADD CONSTRAINT FK_Object_Object FOREIGN KEY (OwnerObjectId) REFERENCES exp.Object (ObjectId)"); new SqlExecutor(getSchema()).execute("ALTER TABLE exp.ExperimentRun ADD CONSTRAINT FK_ExperimentRun_ReplacedByRunId FOREIGN KEY (ReplacedByRunId) REFERENCES exp.ExperimentRun (RowId)"); } + + // Delete from exp.Object and associated tables + public static void deleteObjectIds(SQLFragment objectIdClause) + { + SqlExecutor executor = new SqlExecutor(OntologyManager.getExpSchema()); + + LOG.info(" exp.Edge (FromObjectId)"); + executor.execute( + new SQLFragment("DELETE FROM exp.Edge WHERE FromObjectId") + .append(objectIdClause) + ); + LOG.info(" exp.Edge (ToObjectId)"); + executor.execute( + new SQLFragment("DELETE FROM exp.Edge WHERE ToObjectId") + .append(objectIdClause) + ); + LOG.info(" exp.ObjectProperty"); + executor.execute( + new SQLFragment("DELETE FROM exp.ObjectProperty WHERE ObjectId") + .append(objectIdClause) + ); + LOG.info(" exp.Object"); + executor.execute( + new SQLFragment("DELETE FROM exp.Object WHERE ObjectId") + .append(objectIdClause) + ); + } } diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 5a77fef0b52..78b03f46520 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -196,7 +196,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 25.012; + return 25.013; } @Nullable diff --git a/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java index b2370a9ac8e..e15367bf323 100644 --- a/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java @@ -136,27 +136,7 @@ public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilte .append(objectIdClause) ); - // Delete from exp.Object (and associated tables) - LOG.info(" exp.Edge (FromObjectId)"); - executor.execute( - new SQLFragment("DELETE FROM exp.Edge WHERE FromObjectId") - .append(objectIdClause) - ); - LOG.info(" exp.Edge (ToObjectId)"); - executor.execute( - new SQLFragment("DELETE FROM exp.Edge WHERE ToObjectId") - .append(objectIdClause) - ); - LOG.info(" exp.ObjectProperty"); - executor.execute( - new SQLFragment("DELETE FROM exp.ObjectProperty WHERE ObjectId") - .append(objectIdClause) - ); - LOG.info(" exp.Object"); - executor.execute( - new SQLFragment("DELETE FROM exp.Object WHERE ObjectId") - .append(objectIdClause) - ); + ExperimentMigrationSchemaHandler.deleteObjectIds(objectIdClause); } } }