diff --git a/experiment/resources/schemas/dbscripts/postgresql/exp-26.002-26.003.sql b/experiment/resources/schemas/dbscripts/postgresql/exp-26.002-26.003.sql new file mode 100644 index 00000000000..6cdc852fee4 --- /dev/null +++ b/experiment/resources/schemas/dbscripts/postgresql/exp-26.002-26.003.sql @@ -0,0 +1 @@ +SELECT core.executeJavaUpgradeCode('addRowIdToProvisionedDataClassTables'); diff --git a/experiment/resources/schemas/dbscripts/sqlserver/exp-26.002-26.003.sql b/experiment/resources/schemas/dbscripts/sqlserver/exp-26.002-26.003.sql new file mode 100644 index 00000000000..9c57cc9387e --- /dev/null +++ b/experiment/resources/schemas/dbscripts/sqlserver/exp-26.002-26.003.sql @@ -0,0 +1 @@ +EXEC core.executeJavaUpgradeCode 'addRowIdToProvisionedDataClassTables'; diff --git a/experiment/src/client/test/integration/DataClassCrud.ispec.ts b/experiment/src/client/test/integration/DataClassCrud.ispec.ts index 746173f820c..6c0628efa6c 100644 --- a/experiment/src/client/test/integration/DataClassCrud.ispec.ts +++ b/experiment/src/client/test/integration/DataClassCrud.ispec.ts @@ -1,6 +1,6 @@ import { ExperimentCRUDUtils, - generateFieldName, + generateDomainName, getEscapedNameExpression, hookServer, RequestOptions, @@ -12,6 +12,7 @@ import { checkLackDesignerOrReaderPerm, createSource, deleteSourceType, + generateFieldNameForImport, getDataClassRowIdByName, initProject, verifyRequiredLineageInsertUpdate, @@ -518,7 +519,7 @@ describe('Multi Value Text Choice', () => { }); const dataType = 'MVTCReq Source Type'; - const fieldName = generateFieldName(); + const fieldName = generateFieldNameForImport(); const fieldNameInExpression = getEscapedNameExpression((fieldName)); console.log("Selected Required MVTC dataclass name: " + dataType + ", field name: " + fieldName); @@ -818,4 +819,144 @@ describe('Multi Value Text Choice', () => { }); -}); \ No newline at end of file +}); + +describe('Data CRUD', () => { + + it ("Update using different key fields", async () => { + const dataType = generateDomainName(3) + "UpdateKeyFields"; + const fieldName = generateFieldNameForImport(); + console.log("Selected dataclass name: " + dataType + ", field name: " + fieldName); + + // create data class with one field + await server.post('property', 'createDomain', { + kind: 'DataClass', + domainDesign: { name: dataType, fields: [{ name: fieldName }] }, + options: { name: dataType } + }, { ...topFolderOptions, ...designerReaderOptions }).expect(successfulResponse); + + // insert 2 rows data, provide explicit names and a rowId = 1 + const dataName1 = 'KeyData1'; + const dataName2 = 'KeyData2'; + const inserted = await insertDataClassData([ + { name: dataName1, description: 'original1', [fieldName]: 'val1', rowId: 1 }, + { name: dataName2, description: 'original2', [fieldName]: 'val2', rowId: 1 }, + ], dataType, topFolderOptions); + + // verify both rows are inserted with correct name and rowId is not 1 for both rows, record the rowId and lsid for both rows + expect(inserted[0].name).toBe(dataName1); + expect(inserted[1].name).toBe(dataName2); + expect(inserted[0].rowId).not.toBe(1); + expect(inserted[1].rowId).not.toBe(1); + const row1RowId = inserted[0].rowId; + const row1Lsid = inserted[0].lsid; + const row2RowId = inserted[1].rowId; + const row2Lsid = inserted[1].lsid; + + const findRow = (rows: any[], rowId: number) => rows.find(r => caseInsensitive(r, 'RowId') === rowId); + + // update description and fieldName value for both rows using rowId as key, verify update is successful and data are updated correctly + await ExperimentCRUDUtils.updateRows(server, [ + { rowId: row1RowId, description: 'updByRowId1', [fieldName]: 'rowIdVal1' }, + { rowId: row2RowId, description: 'updByRowId2', [fieldName]: 'rowIdVal2' }, + ], 'exp.data', dataType, topFolderOptions, editorUserOptions); + + let rows = await ExperimentCRUDUtils.getRows(server, [row1RowId, row2RowId], 'exp.data', dataType, '*', topFolderOptions, adminOptions); + let row1 = findRow(rows, row1RowId); + let row2 = findRow(rows, row2RowId); + expect(caseInsensitive(row1, 'description')).toBe('updByRowId1'); + expect(caseInsensitive(row1, fieldName)).toBe('rowIdVal1'); + expect(caseInsensitive(row2, 'description')).toBe('updByRowId2'); + expect(caseInsensitive(row2, fieldName)).toBe('rowIdVal2'); + + // update description and fieldName value for both rows using lsid as key, verify update is successful and data are updated correctly + await ExperimentCRUDUtils.updateRows(server, [ + { lsid: row1Lsid, description: 'updByLsid1', [fieldName]: 'lsidVal1' }, + { lsid: row2Lsid, description: 'updByLsid2', [fieldName]: 'lsidVal2' }, + ], 'exp.data', dataType, topFolderOptions, editorUserOptions); + + rows = await ExperimentCRUDUtils.getRows(server, [row1RowId, row2RowId], 'exp.data', dataType, '*', topFolderOptions, adminOptions); + row1 = findRow(rows, row1RowId); + row2 = findRow(rows, row2RowId); + expect(caseInsensitive(row1, 'description')).toBe('updByLsid1'); + expect(caseInsensitive(row1, fieldName)).toBe('lsidVal1'); + expect(caseInsensitive(row2, 'description')).toBe('updByLsid2'); + expect(caseInsensitive(row2, fieldName)).toBe('lsidVal2'); + + // update description and fieldName value, one of the row use lsid as key, the other use rowId, verify update is successful and data are updated correctly + await ExperimentCRUDUtils.updateRows(server, [ + { lsid: row1Lsid, description: 'updMixed1', [fieldName]: 'mixedVal1' }, + { rowId: row2RowId, description: 'updMixed2', [fieldName]: 'mixedVal2' }, + ], 'exp.data', dataType, topFolderOptions, editorUserOptions); + + rows = await ExperimentCRUDUtils.getRows(server, [row1RowId, row2RowId], 'exp.data', dataType, '*', topFolderOptions, adminOptions); + row1 = findRow(rows, row1RowId); + row2 = findRow(rows, row2RowId); + expect(caseInsensitive(row1, 'description')).toBe('updMixed1'); + expect(caseInsensitive(row1, fieldName)).toBe('mixedVal1'); + expect(caseInsensitive(row2, 'description')).toBe('updMixed2'); + expect(caseInsensitive(row2, fieldName)).toBe('mixedVal2'); + + // update names of both rows using lsid as key, verify update is successful and names are updated correctly + const newName1 = 'RenamedByLsid1'; + const newName2 = 'RenamedByLsid2'; + await ExperimentCRUDUtils.updateRows(server, [ + { lsid: row1Lsid, name: newName1 }, + { lsid: row2Lsid, name: newName2 }, + ], 'exp.data', dataType, topFolderOptions, editorUserOptions); + + rows = await ExperimentCRUDUtils.getRows(server, [row1RowId, row2RowId], 'exp.data', dataType, 'RowId,Name', topFolderOptions, adminOptions); + row1 = findRow(rows, row1RowId); + row2 = findRow(rows, row2RowId); + expect(caseInsensitive(row1, 'Name')).toBe(newName1); + expect(caseInsensitive(row2, 'Name')).toBe(newName2); + + // update names of both rows using rowId as key, verify update is successful and names are updated correctly + const newName3 = 'RenamedByRowId1'; + const newName4 = 'RenamedByRowId2'; + await ExperimentCRUDUtils.updateRows(server, [ + { rowId: row1RowId, name: newName3 }, + { rowId: row2RowId, name: newName4 }, + ], 'exp.data', dataType, topFolderOptions, editorUserOptions); + + rows = await ExperimentCRUDUtils.getRows(server, [row1RowId, row2RowId], 'exp.data', dataType, 'RowId,Name', topFolderOptions, adminOptions); + row1 = findRow(rows, row1RowId); + row2 = findRow(rows, row2RowId); + expect(caseInsensitive(row1, 'Name')).toBe(newName3); + expect(caseInsensitive(row2, 'Name')).toBe(newName4); + + // update description and fieldName value from Import with update, the import columns contains name field, verify update is successful and data are updated correctly + const importUpdateText = 'Name\tDescription\t' + fieldName + '\n' + newName3 + '\timportUpd1\timportVal1\n' + newName4 + '\timportUpd2\timportVal2'; + const updateResp = await ExperimentCRUDUtils.importData(server, importUpdateText, dataType, 'UPDATE', topFolderOptions, editorUserOptions); + expect(updateResp.text.indexOf('"success" : true') > -1).toBeTruthy(); + + rows = await ExperimentCRUDUtils.getRows(server, [row1RowId, row2RowId], 'exp.data', dataType, '*', topFolderOptions, adminOptions); + row1 = findRow(rows, row1RowId); + row2 = findRow(rows, row2RowId); + expect(caseInsensitive(row1, 'description')).toBe('importUpd1'); + expect(caseInsensitive(row1, fieldName)).toBe('importVal1'); + expect(caseInsensitive(row2, 'description')).toBe('importUpd2'); + expect(caseInsensitive(row2, fieldName)).toBe('importVal2'); + + // update description and fieldName value from Import with merge. at the same time create a new data. the import columns contain name field, verify update and insert is successful + const newDataName = 'MergedNewData'; + const importMergeText = 'Name\tDescription\t' + fieldName + '\n' + newName3 + '\tmergeUpd1\tmergeVal1\n' + newName4 + '\tmergeUpd2\tmergeVal2\n' + newDataName + '\tmergeNew\tmergeNewVal'; + const mergeResp = await ExperimentCRUDUtils.importData(server, importMergeText, dataType, 'MERGE', topFolderOptions, editorUserOptions); + expect(mergeResp.text.indexOf('"success" : true') > -1).toBeTruthy(); + + rows = await ExperimentCRUDUtils.getRows(server, [row1RowId, row2RowId], 'exp.data', dataType, '*', topFolderOptions, adminOptions); + row1 = findRow(rows, row1RowId); + row2 = findRow(rows, row2RowId); + expect(caseInsensitive(row1, 'description')).toBe('mergeUpd1'); + expect(caseInsensitive(row1, fieldName)).toBe('mergeVal1'); + expect(caseInsensitive(row2, 'description')).toBe('mergeUpd2'); + expect(caseInsensitive(row2, fieldName)).toBe('mergeVal2'); + + // verify new data was created by merge + const newDataRow = await getDataClassDataByName(newDataName, dataType, '*', topFolderOptions, adminOptions); + expect(caseInsensitive(newDataRow, 'Name')).toBe(newDataName); + expect(caseInsensitive(newDataRow, 'description')).toBe('mergeNew'); + expect(caseInsensitive(newDataRow, fieldName)).toBe('mergeNewVal'); + }); + +}); diff --git a/experiment/src/client/test/integration/utils.ts b/experiment/src/client/test/integration/utils.ts index d6fc55d3081..c36789ae5de 100644 --- a/experiment/src/client/test/integration/utils.ts +++ b/experiment/src/client/test/integration/utils.ts @@ -1,5 +1,6 @@ import { ExperimentCRUDUtils, + generateFieldName, IntegrationTestServer, RequestOptions, selectRandomN, @@ -903,3 +904,16 @@ export async function insertRowsExpectError(server: IntegrationTestServer, rows: export async function updateRowsExpectError(server: IntegrationTestServer, rows: any[], schemaName: string, queryName: string, error: string, folderOptions: RequestOptions, userOptions: RequestOptions) { return insertRowsExpectError(server, rows, schemaName, queryName, error, folderOptions, userOptions, true); } + +export function canNameBeUsedInImport(name: string) { + return name.indexOf(',') === -1 && name.indexOf('\t') === -1 && name.indexOf('"') === -1 && name.indexOf('\n') === -1; +} + +export function generateFieldNameForImport(length: number = 10, charset?: string) { + let fieldName = generateFieldName(length, charset); + while (!canNameBeUsedInImport(fieldName)) + { + fieldName = generateFieldName(); + } + return fieldName; +} \ No newline at end of file diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index edb39708c24..9cde2a5c141 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -205,7 +205,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 26.002; + return 26.003; } @Nullable diff --git a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java index 28756ed29e8..fa2c71c4bb8 100644 --- a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java +++ b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java @@ -30,6 +30,7 @@ import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; +import org.labkey.api.data.DeferredUpgrade; import org.labkey.api.data.JdbcType; import org.labkey.api.data.Parameter; import org.labkey.api.data.ParameterMapStatement; @@ -64,6 +65,9 @@ import org.labkey.api.security.roles.SiteAdminRole; import org.labkey.api.settings.AppProps; import org.labkey.api.util.logging.LogHelper; +import org.labkey.experiment.api.DataClass; +import org.labkey.experiment.api.DataClassDomainKind; +import org.labkey.experiment.api.ExpDataClassImpl; import org.labkey.experiment.api.ExpSampleTypeImpl; import org.labkey.experiment.api.ExperimentServiceImpl; import org.labkey.experiment.api.MaterialSource; @@ -74,6 +78,7 @@ import java.sql.Connection; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -509,4 +514,144 @@ public static void fixContainerForMovedSampleFiles(ModuleContext context) } } + /** + * Called from exp-26.002-26.003.sql + * Drop the classid column and add a rowId column to existing provisioned DataClass tables. + */ + @SuppressWarnings("unused") + @DeferredUpgrade + public static void addRowIdToProvisionedDataClassTables(ModuleContext context) + { + if (context.isNewInstall()) + return; + + try (DbScope.Transaction tx = ExperimentService.get().ensureTransaction()) + { + TableInfo source = ExperimentServiceImpl.get().getTinfoDataClass(); + new TableSelector(source, null, null).stream(DataClass.class) + .map(ExpDataClassImpl::new) + .forEach(ExperimentUpgradeCode::upgradeProvisionedDataClassTable); + + tx.commit(); + } + } + + private static void upgradeProvisionedDataClassTable(ExpDataClassImpl dc) + { + Domain domain = dc.getDomain(); + DataClassDomainKind kind = null; + try + { + kind = (DataClassDomainKind) domain.getDomainKind(); + } + catch (IllegalArgumentException e) + { + // pass + } + if (null == kind || null == kind.getStorageSchemaName()) + return; + + DbSchema schema = DataClassDomainKind.getSchema(); + DbScope scope = schema.getScope(); + + StorageProvisioner storageProvisioner = StorageProvisioner.get(); + + storageProvisioner.ensureStorageTable(domain, kind, scope); + domain = PropertyService.get().getDomain(domain.getTypeId()); + assert (null != domain && null != domain.getStorageTableName()); + + SchemaTableInfo provisionedTable = schema.getTable(domain.getStorageTableName()); + if (provisionedTable == null) + { + LOG.error("DataClass '" + dc.getName() + "' (" + dc.getRowId() + ") has no provisioned table."); + return; + } + + // Drop classid column if present + String classIdColumnName = "classid"; + ColumnInfo classIdColumn = provisionedTable.getColumn(FieldKey.fromParts(classIdColumnName)); + if (classIdColumn != null) + { + Set indicesToRemove = new HashSet<>(); + for (var index : provisionedTable.getAllIndices()) + { + if (index.columns().contains(classIdColumn)) + indicesToRemove.add(index.name()); + } + + if (!indicesToRemove.isEmpty()) + StorageProvisionerImpl.get().dropTableIndices(domain, indicesToRemove); + + // Remanufacture a property descriptor that matches the original classid property descriptor. + var spec = new PropertyStorageSpec(classIdColumnName, JdbcType.INTEGER); + PropertyDescriptor pd = new PropertyDescriptor(); + pd.setContainer(dc.getContainer()); + pd.setDatabaseDefaultValue(spec.getDefaultValue()); + pd.setName(spec.getName()); + pd.setJdbcType(spec.getJdbcType(), spec.getSize()); + pd.setNullable(spec.isNullable()); + pd.setMvEnabled(spec.isMvEnabled()); + pd.setPropertyURI(DomainUtil.createUniquePropertyURI(domain.getTypeURI(), null, new CaseInsensitiveHashSet())); + pd.setDescription(spec.getDescription()); + pd.setImportAliases(spec.getImportAliases()); + pd.setScale(spec.getSize()); + DomainPropertyImpl dp = new DomainPropertyImpl((DomainImpl) domain, pd); + + LOG.debug("Dropping classid column from table '{}' for data class '{}' in folder {}.", provisionedTable.getName(), dc.getName(), dc.getContainer().getPath()); + StorageProvisionerImpl.get().dropProperties(domain, Set.of(dp)); + } + + // Add rowId column if not present + ColumnInfo rowIdCol = provisionedTable.getColumn("rowId"); + if (rowIdCol != null) + { + LOG.info("DataClass '{}' ({}) already has 'rowId' column. Skipping.", dc.getName(), dc.getRowId()); + return; + } + + PropertyStorageSpec rowIdProp = new PropertyStorageSpec("rowId", JdbcType.INTEGER); // nullable for initial add + storageProvisioner.addStorageProperties(domain, Collections.singletonList(rowIdProp), true); + LOG.info("DataClass '{}' ({}) added 'rowId' column", dc.getName(), dc.getRowId()); + + // Populate rowId from exp.data using lsid join + fillRowId(dc, domain, scope); + + // Set NOT NULL constraint + SqlExecutor executor = new SqlExecutor(scope); + boolean isPostgreSQL = scope.getSqlDialect().isPostgreSQL(); + if (isPostgreSQL) + executor.execute(new SQLFragment("ALTER TABLE expdataclass.").append(domain.getStorageTableName()).append(" ALTER COLUMN rowId SET NOT NULL")); + else + executor.execute(new SQLFragment("ALTER TABLE expdataclass.").append(domain.getStorageTableName()).append(" ALTER COLUMN rowId INT NOT NULL")); + + // Add indexes back via StorageProvisioner + storageProvisioner.ensureTableIndices(domain); + LOG.info("DataClass '{}' ({}) added unique index on 'rowId'", dc.getName(), dc.getRowId()); + + // Add FK constraint (no StorageProvisioner API for FKs on existing tables) + String fkName = "fk_rowid_" + domain.getStorageTableName() + "_data"; + executor.execute(new SQLFragment("ALTER TABLE expdataclass.").append(domain.getStorageTableName()) + .append(" ADD CONSTRAINT ").append(fkName) + .append(" FOREIGN KEY (rowId) REFERENCES exp.Data(RowId)")); + LOG.info("DataClass '{}' ({}) added FK constraint on 'rowId'", dc.getName(), dc.getRowId()); + } + + private static void fillRowId(ExpDataClassImpl dc, Domain domain, DbScope scope) + { + String tableName = domain.getStorageTableName(); + SQLFragment update = new SQLFragment() + .append("UPDATE expdataclass.").append(tableName).append("\n") + .append("SET rowId = i.rowid\n") + .append("FROM (\n") + .append(" SELECT d.lsid, d.RowId\n") + .append(" FROM exp.data d\n") + .append(" WHERE d.cpasType = ?\n").add(domain.getTypeURI()) + .append(") AS i\n") + .append("WHERE i.lsid = ").append(tableName).append(".lsid"); + + int count = new SqlExecutor(scope).execute(update); + LOG.info("DataClass '{}' ({}) populated 'rowId' column, count={}", dc.getName(), dc.getRowId(), count); + } + + } diff --git a/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java b/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java index af1d3f26651..63155e503b2 100644 --- a/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java +++ b/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java @@ -99,9 +99,9 @@ public class DataClassDomainKind extends AbstractDomainKind names = new HashSet<>(); @@ -116,13 +116,15 @@ public class DataClassDomainKind extends AbstractDomainKind ALLOWED_IMPORT_HEADERS; static { DATA_CLASS_ALT_MERGE_KEYS = new HashSet<>(Arrays.asList(Column.ClassId.name(), Name.name())); - DATA_CLASS_ALT_UPDATE_KEYS = new HashSet<>(Arrays.asList(Column.LSID.name())); + DATA_CLASS_ALT_UPDATE_KEYS = new HashSet<>(Arrays.asList(Column.LSID.name(), Column.RowId.name())); ALLOWED_IMPORT_HEADERS = new HashSet<>(Arrays.asList("name", "description", "flag", "comment", "alias", "datafileurl")); } @@ -411,7 +411,7 @@ protected void populateColumns() if (null != getDomain()) { TableInfo extTable = _dataClassDataTableSupplier.get(); - Set skipCols = CaseInsensitiveHashSet.of("lsid", "rowid", "name", "classid"); + Set skipCols = CaseInsensitiveHashSet.of("lsid", "rowid", "name"); for (ColumnInfo col : extTable.getColumns()) { // Don't include PHI columns in full text search index @@ -686,11 +686,11 @@ public SQLFragment getFromSQLExpanded(String alias, Set selectedColumn Set dataCols = new CaseInsensitiveHashSet(_rootTable.getColumnNameSet()); - // all columns from dataclass property table except name, lsid, and classid + // all columns from dataclass property table except name, lsid Set pCols = new CaseInsensitiveHashSet(provisioned.getColumnNameSet()); pCols.remove("name"); pCols.remove("lsid"); - pCols.remove("classid"); + pCols.remove("rowId"); boolean hasProvisionedColumns = containsProvisionedColumns(selectedColumns, pCols); @@ -716,7 +716,7 @@ public SQLFragment getFromSQLExpanded(String alias, Set selectedColumn sql.append(" FROM "); sql.append(_rootTable, "d"); if (hasProvisionedColumns) - sql.append(" INNER JOIN ").append(provisioned, "p").append(" ON d.lsid = p.lsid"); + sql.append(" INNER JOIN ").append(provisioned, "p").append(" ON d.RowId = p.rowId"); String subAlias = getSqlDialect().truncate(alias + "_dc_sub", 0); sql.append(") ").appendIdentifier(subAlias); sql.append("\n"); @@ -792,32 +792,7 @@ public List getUniqueIndices() { List indices = new ArrayList<>(super.getUniqueIndices()); indices.addAll(wrapTableIndices(_dataClassDataTableSupplier.get())); - - // Issue 46948: RemapCache unable to resolve ExpData objects with addition of ClassId column - // RemapCache is used to findExpData using name/rowId remap. - // The addition of "ClassId" column to the TableInfo is causing violation of RemapCache's requirement of "unique index over a single column that isn't the primary key". - // Because this is a joined table between exp.data and the dataclass provisioned table, it's safe to ignore "ClassId" as part of the unique key. - List filteredIndices = new ArrayList<>(); - for (IndexDefinition def : indices) - { - IndexType type = def.indexType(); - List columns = def.columns(); - - List filteredColumns = new ArrayList<>(); - if (type == IndexType.Unique && columns.size() > 1) - { - for (ColumnInfo columnInfo : columns) - { - if (Column.ClassId.name().equalsIgnoreCase(columnInfo.getName())) - continue; - - filteredColumns.add(columnInfo); - } - } - - filteredIndices.add(new IndexDefinition(def.name(), def.indexType(), filteredColumns.isEmpty() ? columns : filteredColumns, def.filterCondition())); - } - return Collections.unmodifiableList(filteredIndices); + return Collections.unmodifiableList(indices); } @NotNull @@ -908,7 +883,9 @@ public Set getAltKeysForUpdate() context.getConfigParameterBoolean(ExperimentService.QueryOptions.UseLsidForUpdate); if (isUpdateUsingLsid) + { keyColumnNames.add(Column.LSID.name()); + } else { Set altMergeKeys = getAltMergeKeys(context); @@ -916,8 +893,6 @@ public Set getAltKeysForUpdate() keyColumnNames.addAll(altMergeKeys); } } - else - keyColumnNames.add(Column.LSID.name()); return keyColumnNames.isEmpty() ? null : keyColumnNames; } @@ -1054,8 +1029,7 @@ else if (Column.ClassId.name().equalsIgnoreCase(name)) TableInfo expData = svc.getTinfoData(); // Ensure we have a dataClass column and it is of the right value - // use materialized classId so that parameter binding works for both exp.data as well as materialized table - ColumnInfo classIdCol = _dataClassDataTableSupplier.get().getColumn("classId"); + ColumnInfo classIdCol = expData.getColumn("classId"); step0.addColumn(classIdCol, new SimpleTranslator.ConstantColumn(_dataClass.getRowId())); // Ensure we have a cpasType column and it is of the right value @@ -1505,12 +1479,14 @@ else if (value != null && !StringUtils.isEmpty(String.valueOf(value))) // update exp.data Map ret = new CaseInsensitiveHashMap<>(super._update(user, c, rowStripped, oldRow, keys)); - // update provisioned table -- note that LSID isn't the PK so we need to use the filter to update the correct row instead - keys = new Object[] {lsid}; + Integer rowId = (Integer) oldRow.get("RowId"); + if (rowId == null) + throw new ValidationException("RowId required to update row"); + keys = new Object[] {rowId}; TableInfo t = _dataClassDataTableSupplier.get(); if (t.getColumnNameSet().stream().anyMatch(rowStripped::containsKey)) { - ret.putAll(Table.update(user, t, rowStripped, t.getColumn("lsid"), keys, null, Level.DEBUG)); + ret.putAll(Table.update(user, t, rowStripped, t.getColumn("rowId"), keys, null, Level.DEBUG)); } ExpDataImpl data = null; @@ -1679,11 +1655,7 @@ public void configureDataIteratorContext(DataIteratorContext context) if (context.getInsertOption().allowUpdate) context.putConfigParameter(QueryUpdateService.ConfigParameters.CheckForCrossProjectData, true); if (context.getInsertOption() == InsertOption.IMPORT || context.getInsertOption() == InsertOption.MERGE) - { - AuditBehaviorType auditType = (AuditBehaviorType) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior); - if (auditType != null && auditType != AuditBehaviorType.NONE) - context.setSelectIds(true); // select rowId for QueryUpdateAuditEvent.rowPk - } + context.setSelectIds(true); // select rowId because provisioned expdataclass.rowId and QueryUpdateAuditEvent.rowPk needs actual rowId } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index b34a95cc8ef..c61e388703f 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -1289,7 +1289,7 @@ private void indexDataClassData(ExpDataClassImpl dataClass, SearchService.TaskIn SQLFragment sql = new SQLFragment() .append("SELECT * FROM ").append(getTinfoData(), "d") .append(" INNER JOIN ").append(table, "t") - .append(" ON t.lsid = d.lsid") + .append(" ON t.rowId = d.RowId") .append(" LEFT OUTER JOIN ").append(getTinfoDataIndexed(), "di") .append(" ON d.RowId = di.DataId") .append(" WHERE d.classId = ?").add(dataClass.getRowId()) @@ -1846,7 +1846,7 @@ public List getExpDatas(ExpDataClass dataClass) SQLFragment sql = new SQLFragment() .append("SELECT * FROM ").append(getTinfoData(), "d") .append(", ").append(table, "t") - .append(" WHERE t.lsid = d.lsid") + .append(" WHERE t.rowId = d.RowId") .append(" AND d.classId = ?").add(dataClass.getRowId()); List datas = new SqlSelector(table.getSchema().getScope(), sql).getArrayList(Data.class); @@ -1871,7 +1871,7 @@ public ExpDataImpl getExpData(ExpDataClass dataClass, String name) SQLFragment sql = new SQLFragment() .append("SELECT * FROM ").append(getTinfoData(), "d") .append(", ").append(table, "t") - .append(" WHERE t.lsid = d.lsid") + .append(" WHERE t.rowId = d.RowId") .append(" AND d.classId = ?").add(dataClass.getRowId()) .append(" AND d.Name = ?").add(name); @@ -1889,7 +1889,7 @@ public ExpDataImpl getExpData(ExpDataClass dataClass, long rowId) SQLFragment sql = new SQLFragment() .append("SELECT * FROM ").append(getTinfoData(), "d") .append(", ").append(table, "t") - .append(" WHERE t.lsid = d.lsid") + .append(" WHERE t.rowId = d.RowId") .append(" AND d.classId = ?").add(dataClass.getRowId()) .append(" AND d.rowId = ?").add(rowId); @@ -5307,7 +5307,7 @@ public void deleteDataByRowIds(User user, Container container, Collection } List allLsids = new ArrayList<>(datas.size()); - Map> lsidsByClass = new LinkedHashMap<>(); + Map> rowIdsByClass = new LinkedHashMap<>(); for (Data data : datas) { @@ -5340,8 +5340,8 @@ public void deleteDataByRowIds(User user, Container container, Collection if (data.getClassId() != null) { - List byClass = lsidsByClass.computeIfAbsent(data.getClassId(), k -> new ArrayList<>(10)); - byClass.add(data.getLSID()); + List byClass = rowIdsByClass.computeIfAbsent(data.getClassId(), k -> new ArrayList<>(10)); + byClass.add(data.getRowId()); } allLsids.add(data.getLSID()); } @@ -5368,18 +5368,18 @@ public void deleteDataByRowIds(User user, Container container, Collection // exp.DataIndexed handled via a ON DELETE CASCADE foreign key // DELETE FROM provisioned dataclass tables - for (var classId : lsidsByClass.keySet()) + for (var classId : rowIdsByClass.keySet()) { ExpDataClassImpl dataClass = getDataClass(classId); if (dataClass == null) throw new SQLException("DataClass not found '" + classId + "'"); - List lsids = lsidsByClass.get(classId); - if (!lsids.isEmpty()) + List rowIds = rowIdsByClass.get(classId); + if (!rowIds.isEmpty()) { TableInfo t = dataClass.getTinfo(); - SQLFragment sql = new SQLFragment("DELETE FROM ").append(t).append(" WHERE lsid "); - dialect.appendInClauseSql(sql, lsids); + SQLFragment sql = new SQLFragment("DELETE FROM ").append(t).append(" WHERE rowId "); + dialect.appendInClauseSql(sql, rowIds); executor.execute(sql); } } @@ -6098,9 +6098,9 @@ private void truncateDataClassAttachments(ExpDataClassImpl dataClass) TableInfo table = dataClass.getTinfo(); SQLFragment sql = new SQLFragment() - .append("SELECT t.lsid FROM ").append(getTinfoData(), "d") + .append("SELECT d.lsid FROM ").append(getTinfoData(), "d") .append(" LEFT OUTER JOIN ").append(table, "t") - .append(" ON d.lsid = t.lsid") + .append(" ON d.RowId = t.rowId") .append(" WHERE d.Container = ?").add(dataClass.getContainer().getEntityId()) .append(" AND d.ClassId = ?").add(dataClass.getRowId());