Rework sequence querying & updating

api/src/org/labkey/api/data/DatabaseMigrationService.java

@@ -4,7 +4,6 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.labkey.api.data.DatabaseMigrationConfiguration.DefaultDatabaseMigrationConfiguration;
-import org.labkey.api.data.DatabaseMigrationService.MigrationSchemaHandler.Sequence;
 import org.labkey.api.data.SimpleFilter.AndClause;
 import org.labkey.api.data.SimpleFilter.FilterClause;
 import org.labkey.api.data.SimpleFilter.InClause;
@@ -16,15 +15,16 @@
 import org.labkey.api.services.ServiceRegistry;
 import org.labkey.api.util.ConfigurationException;
 import org.labkey.api.util.GUID;
+import org.labkey.api.util.StringUtilsLabKey;
 import org.labkey.api.util.logging.LogHelper;
 import org.labkey.vfs.FileLike;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -66,18 +66,13 @@ default void registerMigrationFilter(MigrationFilter filter) {}
         return null;
     }
 
-    default void copySourceTableToTargetTable(DatabaseMigrationConfiguration configuration, TableInfo sourceTable, TableInfo targetTable, DbSchemaType schemaType, Map<String, Sequence> schemaSequenceMap, MigrationSchemaHandler schemaHandler) {};
+    default void copySourceTableToTargetTable(DatabaseMigrationConfiguration configuration, TableInfo sourceTable, TableInfo targetTable, DbSchemaType schemaType, MigrationSchemaHandler schemaHandler) {};
 
     interface MigrationSchemaHandler
     {
-        record Sequence(String schemaName, String tableName, String columnName, long lastValue) {}
-
         // Marker for tables to declare themselves as site-wide (no container filtering)
         FieldKey SITE_WIDE_TABLE = FieldKey.fromParts("site-wide");
 
-        // Dummy value returned from getContainerFieldKey() to ensure that custom getContainerClause() method is called
-        FieldKey DUMMY_FIELD_KEY = FieldKey.fromParts("DUMMY");
-
         DbSchema getSchema();
 
         void beforeVerification();
@@ -87,27 +82,27 @@ record Sequence(String schemaName, String tableName, String columnName, long las
         List<TableInfo> getTablesToCopy();
 
         // Create a filter clause that selects from all specified containers and (in some overrides) applies table-specific filters
-        FilterClause getTableFilter(TableInfo sourceTable, FieldKey containerFieldKey, Set<GUID> containers);
+        FilterClause getTableFilterClause(TableInfo sourceTable, Set<GUID> containers);
 
         // Create a filter clause that selects from all specified containers
-        FilterClause getContainerClause(TableInfo sourceTable, FieldKey containerFieldKey, Set<GUID> containers);
+        FilterClause getContainerClause(TableInfo sourceTable, Set<GUID> containers);
 
         // Return the FieldKey that can be used to filter this table by container. Special values SITE_WIDE_TABLE and
         // DUMMY_FIELD_KEY can be returned for special behaviors. DUMMY_FIELD_KEY ensures that the handler's custom
         // getContainerClause() is always called. SITE_WIDE_TABLE is used to select all rows.
         @Nullable FieldKey getContainerFieldKey(TableInfo sourceTable);
 
         // Create a filter clause that selects all rows from unfiltered containers plus filtered rows from the filtered containers
-        FilterClause getDomainDataFilter(Set<GUID> copyContainers, Set<GUID> filteredContainers, List<DataFilter> domainFilters, TableInfo sourceTable, FieldKey containerFieldKey, Set<String> selectColumnNames);
+        FilterClause getDomainDataFilterClause(Set<GUID> copyContainers, Set<GUID> filteredContainers, List<DataFilter> domainFilters, TableInfo sourceTable, Set<String> selectColumnNames);
 
-        void addDomainDataFilter(OrClause orClause, DataFilter filter, TableInfo sourceTable, FieldKey fKey, Set<String> selectColumnNames);
+        void addDomainDataFilterClause(OrClause orClause, DataFilter filter, TableInfo sourceTable, Set<String> selectColumnNames);
 
         // Do any necessary clean up after the target table has been populated. notCopiedFilter selects all rows in the
         // source table that were NOT copied to the target table. (For example, rows in a global table not copied due to
         // container filtering or rows in a provisioned table not copied due to domain data filtering.)
         void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter);
 
-        void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, DbSchema targetSchema, Map<String, Map<String, Sequence>> sequenceMap);
+        void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, DbSchema targetSchema);
     }
 
     class DefaultMigrationSchemaHandler implements MigrationSchemaHandler
@@ -157,16 +152,18 @@ public List<TableInfo> getTablesToCopy()
         }
 
         @Override
-        public FilterClause getTableFilter(TableInfo sourceTable, FieldKey containerFieldKey, Set<GUID> containers)
+        public FilterClause getTableFilterClause(TableInfo sourceTable, Set<GUID> containers)
         {
-            return getContainerClause(sourceTable, containerFieldKey, containers);
+            return getContainerClause(sourceTable, containers);
         }
 
         @Override
-        public FilterClause getContainerClause(TableInfo sourceTable, FieldKey containerFieldKey, Set<GUID> containers)
+        public FilterClause getContainerClause(TableInfo sourceTable, Set<GUID> containers)
         {
-            if (containerFieldKey == SITE_WIDE_TABLE || containerFieldKey == DUMMY_FIELD_KEY)
-                throw new IllegalStateException("Should not be supplying " + containerFieldKey + " to the default getContainerClause() method");
+            FieldKey containerFieldKey = getContainerFieldKey(sourceTable);
+
+            if (containerFieldKey == SITE_WIDE_TABLE)
+                return new SQLClause(new SQLFragment("TRUE"));
 
             return new InClause(containerFieldKey, containers);
         }
@@ -213,17 +210,17 @@ public FilterClause getContainerClause(TableInfo sourceTable, FieldKey container
         }
 
         @Override
-        public FilterClause getDomainDataFilter(Set<GUID> copyContainers, Set<GUID> filteredContainers, List<DataFilter> domainFilters, TableInfo sourceTable, FieldKey fKey, Set<String> selectColumnNames)
+        public final FilterClause getDomainDataFilterClause(Set<GUID> copyContainers, Set<GUID> filteredContainers, List<DataFilter> domainFilters, TableInfo sourceTable, Set<String> selectColumnNames)
         {
             // Filtered case: remove the filtered containers from the unconditional container set
             Set<GUID> otherContainers = new HashSet<>(copyContainers);
             otherContainers.removeAll(filteredContainers);
-            FilterClause ret = getContainerClause(sourceTable, fKey, otherContainers);
+            FilterClause ret = getContainerClause(sourceTable, otherContainers);
 
             OrClause orClause = new OrClause();
 
             // Delegate to the MigrationSchemaHandler to add domain-filtered containers back with their special filter applied
-            domainFilters.forEach(filter -> addDomainDataFilter(orClause, filter, sourceTable, fKey, selectColumnNames));
+            domainFilters.forEach(filter -> addDomainDataFilterClause(orClause, filter, sourceTable, selectColumnNames));
 
             if (!orClause.getClauses().isEmpty())
             {
@@ -235,13 +232,13 @@ public FilterClause getDomainDataFilter(Set<GUID> copyContainers, Set<GUID> filt
         }
 
         @Override
-        public void addDomainDataFilter(OrClause orClause, DataFilter filter, TableInfo sourceTable, FieldKey fKey, Set<String> selectColumnNames)
+        public void addDomainDataFilterClause(OrClause orClause, DataFilter filter, TableInfo sourceTable, Set<String> selectColumnNames)
         {
-            addDataFilter(orClause, filter, sourceTable, fKey, selectColumnNames);
+            addDataFilterClause(orClause, filter, sourceTable, selectColumnNames);
         }
 
         // Add a filter and return true if the column exists directly on the table
-        protected boolean addDataFilter(OrClause orClause, DataFilter filter, TableInfo sourceTable, FieldKey fKey, Set<String> selectColumnNames)
+        protected boolean addDataFilterClause(OrClause orClause, DataFilter filter, TableInfo sourceTable, Set<String> selectColumnNames)
         {
             boolean columnExists = selectColumnNames.contains(filter.column());
 
@@ -250,7 +247,7 @@ protected boolean addDataFilter(OrClause orClause, DataFilter filter, TableInfo
                 // Select all rows in this domain-filtered container that meet its criteria
                 orClause.addClause(
                     new AndClause(
-                        getContainerClause(sourceTable, fKey, filter.containers()),
+                        getContainerClause(sourceTable, filter.containers()),
                         filter.condition()
                     )
                 );
@@ -259,32 +256,20 @@ protected boolean addDataFilter(OrClause orClause, DataFilter filter, TableInfo
             return columnExists;
         }
 
-        // Add a filter to select all rows where the object property with <propertyId> equals the filter value
-        protected void addObjectPropertyFilter(OrClause orClause, DataFilter filter, TableInfo sourceTable, FieldKey fKey, int propertyId)
+        // Add a clause that selects all rows where the object property with <propertyId> equals the filter value. This
+        // is only for provisioned tables that lack an ObjectId, MaterialId, or DataId column.
+        protected void addObjectPropertyClause(OrClause orClause, DataFilter filter, TableInfo sourceTable, int propertyId)
         {
-            SQLFragment flagWhere = new SQLFragment("lsid IN (SELECT ObjectURI FROM exp.Object WHERE ObjectId IN (SELECT ObjectId FROM exp.ObjectProperty WHERE StringValue = ? AND PropertyId = ?))", filter.condition().getParamVals()[0], propertyId);
+            SQLFragment flagWhere = new SQLFragment("lsid IN (SELECT ObjectURI FROM exp.Object o INNER JOIN exp.ObjectProperty op ON o.ObjectId = op.ObjectId WHERE StringValue = ? AND PropertyId = ?)", filter.condition().getParamVals()[0], propertyId);
 
             orClause.addClause(
                 new AndClause(
-                    getContainerClause(sourceTable, fKey, filter.containers()),
+                    getContainerClause(sourceTable, filter.containers()),
                     new SQLClause(flagWhere)
                 )
             );
         }
 
-        // Special domain data filter method for provisioned tables that have a built-in Flag field (currently used by data classes)
-        protected void addDomainDataFlagFilter(OrClause orClause, DataFilter filter, TableInfo sourceTable, FieldKey fKey, Set<String> selectColumnNames)
-        {
-            if (filter.column().equalsIgnoreCase("Flag"))
-            {
-                addObjectPropertyFilter(orClause, filter, sourceTable, fKey, getCommentPropertyId(sourceTable.getSchema().getScope()));
-            }
-            else
-            {
-                addDataFilter(orClause, filter, sourceTable, fKey, selectColumnNames);
-            }
-        }
-
         private Integer _commentPropertyId = null;
 
         protected synchronized int getCommentPropertyId(DbScope scope)
@@ -304,13 +289,18 @@ protected synchronized int getCommentPropertyId(DbScope scope)
             return _commentPropertyId;
         }
 
+        protected String rowsNotCopied(int count)
+        {
+            return "   " + StringUtilsLabKey.pluralize(count, "row") + " not copied";
+        }
+
         @Override
         public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter)
         {
         }
 
         @Override
-        public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, DbSchema targetSchema, Map<String, Map<String, Sequence>> sequenceMap)
+        public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, DbSchema targetSchema)
         {
         }
     }
@@ -339,6 +329,27 @@ interface MigrationFilter
         void saveFilter(@Nullable GUID guid, String value);
     }
 
+    interface ExperimentDeleteService
+    {
+        static @NotNull ExperimentDeleteService get()
+        {
+            ExperimentDeleteService ret = ServiceRegistry.get().getService(ExperimentDeleteService.class);
+            if (ret == null)
+                throw new IllegalStateException("ExperimentDeleteService not found");
+            return ret;
+        }
+
+        static void setInstance(ExperimentDeleteService impl)
+        {
+            ServiceRegistry.get().registerService(ExperimentDeleteService.class, impl);
+        }
+
+        /**
+         * Deletes all rows from exp.Data, exp.Object, and related tables associated with the provided ObjectIds
+         */
+        void deleteDataRows(Collection<Long> objectIds);
+    }
+
     // Helper method that parses a data filter then adds it and its container to the provided collections, coalescing
     // cases where multiple containers specify the same filter
     static void addDataFilter(String filterName, List<DataFilter> dataFilters, Set<GUID> filteredContainers, GUID guid, String filter)

api/src/org/labkey/api/data/dialect/SqlDialect.java

@@ -2093,9 +2093,11 @@ public boolean shouldTest()
         return null;
     }
 
-    // Returns a SQL query that selects the last auto-increment values where they're non-null. Required columns are:
-    // SchemaName, TableName, ColumnName, and LastValue.
-    public String getSelectSequencesSql()
+    public record Sequence(String schemaName, String tableName, String columnName, Long lastValue) {}
+
+    // Returns information about all auto-increment / serial sequences associated with a table. PostgreSQL tables can
+    // have more than one. Sequence value will be null if the sequence hasn't been incremented yet.
+    public @NotNull Collection<Sequence> getAutoIncrementSequences(TableInfo table)
     {
         throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement");
     }

api/src/org/labkey/api/dataiterator/StatementDataIterator.java

@@ -129,8 +129,8 @@ public void setUseAsynchronousExecute(boolean useAsynchronousExecute)
      * does not 'release' rows until the statement that operates on that row (e.g. inserts it) has been
      * executed.
      *
-     * This is different than the normal flow of control where 'later' data iterators only call 'earlier' data iterators.
-     * In this case the StatementDataIterator is passing some internal state information forward to to the EmbargoDataIterator
+     * This is different from the normal flow of control where 'later' data iterators only call 'earlier' data iterators.
+     * In this case the StatementDataIterator is passing some internal state information forward to the EmbargoDataIterator
      * This is actually fine, since it's the DataIteratorBuilder's job to set up a correct pipeline.
      */
     public void setEmbargoDataIterator(EmbargoDataIterator cache)

assay/src/org/labkey/assay/AssayModule.java

@@ -307,6 +307,8 @@ public void moduleStartupComplete(ServletContext servletContext)
                 return SITE_WIDE_TABLE;
             }
         });
+
+        DatabaseMigrationService.get().registerSchemaHandler(new AssayResultMigrationSchemaHandler());
     }
 
     @Override
@@ -319,9 +321,8 @@ public ActionURL getTabURL(Container c, User user)
     @NotNull
     public Set<String> getSchemaNames()
     {
-        HashSet<String> set = new HashSet<>();
+        HashSet<String> set = new HashSet<>(getProvisionedSchemaNames());
         set.add(AssayDbSchema.getInstance().getSchemaName());
-        set.addAll(getProvisionedSchemaNames());
 
         return set;
     }
@@ -331,8 +332,8 @@ public Set<String> getSchemaNames()
     public Set<String> getProvisionedSchemaNames()
     {
         return Set.of(
-                AbstractTsvAssayProvider.ASSAY_SCHEMA_NAME,
-                PlateMetadataDomainKind.PROVISIONED_SCHEMA_NAME
+            AbstractTsvAssayProvider.ASSAY_SCHEMA_NAME,
+            PlateMetadataDomainKind.PROVISIONED_SCHEMA_NAME
         );
     }
 
@@ -358,13 +359,13 @@ public Set<String> getProvisionedSchemaNames()
     public @NotNull Set<Class<?>> getUnitTests()
     {
         return Set.of(
-            TsvAssayProvider.TestCase.class,
-            AssaySchemaImpl.TestCase.class,
+            AssayPlateMetadataServiceImpl.TestCase.class,
             AssayProviderSchema.TestCase.class,
-            PositionImpl.TestCase.class,
+            AssaySchemaImpl.TestCase.class,
             PlateImpl.TestCase.class,
             PlateUtils.TestCase.class,
-            AssayPlateMetadataServiceImpl.TestCase.class
+            PositionImpl.TestCase.class,
+            TsvAssayProvider.TestCase.class
         );
     }
 

assay/src/org/labkey/assay/AssayResultMigrationSchemaHandler.java

@@ -0,0 +1,80 @@
+package org.labkey.assay;
+
+import org.apache.logging.log4j.Logger;
+import org.labkey.api.assay.AbstractTsvAssayProvider;
+import org.labkey.api.data.DatabaseMigrationService.DataFilter;
+import org.labkey.api.data.DatabaseMigrationService.DefaultMigrationSchemaHandler;
+import org.labkey.api.data.DatabaseMigrationService.ExperimentDeleteService;
+import org.labkey.api.data.DbSchema;
+import org.labkey.api.data.DbSchemaType;
+import org.labkey.api.data.SQLFragment;
+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.SqlSelector;
+import org.labkey.api.data.TableInfo;
+import org.labkey.api.util.GUID;
+import org.labkey.api.util.logging.LogHelper;
+
+import java.util.Collection;
+import java.util.Set;
+
+class AssayResultMigrationSchemaHandler extends DefaultMigrationSchemaHandler
+{
+    private static final Logger LOG = LogHelper.getLogger(AssayResultMigrationSchemaHandler.class, "Assay result migration status");
+
+    public AssayResultMigrationSchemaHandler()
+    {
+        super(DbSchema.get(AbstractTsvAssayProvider.ASSAY_SCHEMA_NAME, DbSchemaType.Provisioned));
+    }
+
+    // Provisioned assay result tables occasionally have no DataId column; hopefully they have an LSID column.
+    private boolean hasDataIdColumn(TableInfo sourceTable)
+    {
+        return sourceTable.getColumn("DataId") != null;
+    }
+
+    @Override
+    public FilterClause getContainerClause(TableInfo sourceTable, Set<GUID> containers)
+    {
+        return new SQLClause(
+            new SQLFragment(hasDataIdColumn(sourceTable) ? "DataId IN (SELECT RowId" : "LSID IN (SELECT LSID")
+                .append(" FROM exp.Data WHERE Container")
+                .appendInClause(containers, sourceTable.getSqlDialect())
+                .append(")")
+        );
+    }
+
+    @Override
+    public void addDomainDataFilterClause(OrClause orClause, DataFilter filter, TableInfo sourceTable, Set<String> selectColumnNames)
+    {
+        // We want no rows from containers with a domain data filter, so don't add any clauses
+    }
+
+    @Override
+    public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter)
+    {
+        SQLFragment objectIdSql = new SQLFragment("SELECT ObjectId FROM exp.Data WHERE ")
+            .append(hasDataIdColumn(sourceTable) ? "RowId IN (SELECT DataId" : "LSID IN (SELECT LSID")
+            .append(" FROM ")
+            .appendIdentifier(sourceTable.getSelectName())
+            .append(" ")
+            .append(notCopiedFilter.getSQLFragment(sourceTable.getSqlDialect()))
+            .append(")");
+
+        Collection<Long> notCopiedObjectIds = new SqlSelector(sourceTable.getSchema(), objectIdSql).getCollection(Long.class);
+
+        if (notCopiedObjectIds.isEmpty())
+        {
+            LOG.info(rowsNotCopied(0));
+        }
+        else
+        {
+            LOG.info("{} -- deleting associated rows from exp.Data, exp.Object, etc.", rowsNotCopied(notCopiedObjectIds.size()));
+
+            // Delete exp.Data, exp.Object, etc. rows associated with the rows that weren't copied
+            ExperimentDeleteService.get().deleteDataRows(notCopiedObjectIds);
+        }
+    }
+}