diff --git a/api/src/org/labkey/api/data/PropertyStorageSpec.java b/api/src/org/labkey/api/data/PropertyStorageSpec.java index 66a40909c0e..2f7c9667399 100644 --- a/api/src/org/labkey/api/data/PropertyStorageSpec.java +++ b/api/src/org/labkey/api/data/PropertyStorageSpec.java @@ -431,6 +431,7 @@ public static class Index { final public String[] columnNames; final public boolean isUnique; + final public boolean isCaseInsensitive; // use for postgres only final public boolean isClustered; public Index(boolean unique, String... columnNames) @@ -438,6 +439,7 @@ public Index(boolean unique, String... columnNames) this.columnNames = columnNames; this.isUnique = unique; this.isClustered = false; + this.isCaseInsensitive = false; } /** If the set of names refers to propertydescriptors, then the PropertyDescriptors must be provided */ @@ -451,7 +453,7 @@ public Index translateToStorageNames(Domain domain) throw new IllegalStateException("Property " + propertyName + " has no storage column name"); return null == pd ? propertyName : pd.getStorageColumnName(); }).toArray(String[]::new); - return new Index(isUnique, isClustered, translatedNames); + return new Index(isUnique, isClustered, isCaseInsensitive, translatedNames); } public Index(boolean unique, Collection columnNames) @@ -459,6 +461,7 @@ public Index(boolean unique, Collection columnNames) this.columnNames = columnNames.toArray(new String[0]); this.isUnique = unique; this.isClustered = false; + this.isCaseInsensitive = false; } public Index(boolean unique, boolean clustered, String... columnNames) @@ -466,6 +469,15 @@ public Index(boolean unique, boolean clustered, String... columnNames) this.columnNames = columnNames; this.isUnique = unique; this.isClustered = clustered; + this.isCaseInsensitive = false; + } + + public Index(boolean unique, boolean clustered, boolean caseInsensitive, String... columnNames) + { + this.columnNames = columnNames; + this.isUnique = unique; + this.isClustered = clustered; + this.isCaseInsensitive = caseInsensitive; } /** @@ -474,7 +486,7 @@ public Index(boolean unique, boolean clustered, String... columnNames) */ public static boolean isSameIndex(Index propertyIndex, Index tableIndex) { - if (propertyIndex.isUnique != tableIndex.isUnique || propertyIndex.columnNames.length != tableIndex.columnNames.length) + if (propertyIndex.isCaseInsensitive != tableIndex.isCaseInsensitive || propertyIndex.isUnique != tableIndex.isUnique || propertyIndex.columnNames.length != tableIndex.columnNames.length) return false; Set piColumns = new CaseInsensitiveHashSet(Arrays.asList(propertyIndex.columnNames)); Set tiColumns = new CaseInsensitiveHashSet(Arrays.asList(tableIndex.columnNames)); diff --git a/api/src/org/labkey/api/data/SchemaColumnMetaData.java b/api/src/org/labkey/api/data/SchemaColumnMetaData.java index cba8e1638f2..88339766c5e 100644 --- a/api/src/org/labkey/api/data/SchemaColumnMetaData.java +++ b/api/src/org/labkey/api/data/SchemaColumnMetaData.java @@ -364,6 +364,13 @@ private void loadIndices(SchemaTableInfo ti) throws SQLException } ColumnInfo colInfo = getColumn(colName); + if (colInfo == null && colName.startsWith("lower(")) + { + colInfo = getColumn(colName.substring("lower(".length(), colName.length() - 1)); + if (colInfo == null && colName.endsWith(")::text)")) + colInfo = getColumn(colName.substring("lower((".length(), colName.length() - ")::text)".length())); + } + // Column will be null for indices over expressions, eg.: "lower(name)" if (colInfo == null) ignoreIndex.add(indexName); diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index f341051b1e8..662e7af7f78 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -1182,8 +1182,8 @@ private List getRenameColumnsStatement(TableChange change) { PropertyStorageSpec.Index oldIndex = oldToNew.getKey(); PropertyStorageSpec.Index newIndex = oldToNew.getValue(); - String oldName = nameIndex(change.getTableName(), oldIndex.columnNames); - String newName = nameIndex(change.getTableName(), newIndex.columnNames); + String oldName = nameIndex(change.getTableName(), oldIndex); + String newName = nameIndex(change.getTableName(), newIndex); if (!oldName.equals(newName)) { statements.add(String.format("ALTER INDEX %s.%s RENAME TO %s", @@ -1336,22 +1336,23 @@ private void addCreateIndexStatements(List statements, TableChange chang { statements.add(String.format("CREATE %sINDEX %s ON %s (%s);", index.isUnique ? "UNIQUE " : "", - nameIndex(change.getTableName(), index.columnNames), + nameIndex(change.getTableName(), index), makeTableIdentifier(change), - makePropertyIdentifiers(index.columnNames))); + makePropertyIdentifiers(index))); if (index.isClustered) { statements.add(String.format("%s %s.%s USING %s", PropertyStorageSpec.CLUSTER_TYPE.CLUSTER, change.getSchemaName(), - change.getTableName(), nameIndex(change.getTableName(), index.columnNames))); + change.getTableName(), nameIndex(change.getTableName(), index))); } } } @Override - public String nameIndex(String tableName, String[] indexedColumns) + public String nameIndex(String tableName, PropertyStorageSpec.Index index) { - return AliasManager.makeLegalName(tableName + '_' + StringUtils.join(indexedColumns, "_"), this); + String suffix = index.isCaseInsensitive ? "_lower" : ""; + return AliasManager.makeLegalName(tableName + '_' + StringUtils.join(index.columnNames, "_") + suffix, this); } private String getSqlColumnSpec(PropertyStorageSpec prop) @@ -1435,14 +1436,15 @@ else if (prop.getJdbcType() == JdbcType.VARCHAR && (prop.getSize() == -1 || prop } } - // Create comma-separated list of property identifiers - private String makePropertyIdentifiers(String[] names) + // Create comma-separated list of property identifiers for index + private String makePropertyIdentifiers(PropertyStorageSpec.Index index) { String sep = ""; StringBuilder sb = new StringBuilder(); - for (String name : names) + boolean isCaseInsensitive = index.isCaseInsensitive; + for (String name : index.columnNames) { - sb.append(sep).append(makePropertyIdentifier(name)); + sb.append(sep).append(isCaseInsensitive ? "lower(" : "").append(makePropertyIdentifier(name)).append(isCaseInsensitive ? ")" : ""); sep = ", "; } return sb.toString(); diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index 537929ef88e..48d70502009 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SqlDialect.java @@ -149,7 +149,8 @@ public DatabaseTableType getTableType(String tableTypeName) return description; } - public String nameIndex(String tableName, String[] indexedColumns){ + public String nameIndex(String tableName, PropertyStorageSpec.Index index) + { throw new UnsupportedOperationException("Update " + this.getClass().getSimpleName() + " to add this functionality"); } diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java index e3c55d3286c..039244d2762 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java @@ -152,7 +152,8 @@ public class SampleTypeDomainKind extends AbstractDomainKind newOrExistingName = new HashSet<>(); // use lower case + private final Set newOrExistingNameLc = new HashSet<>(); // keep case protected DuplicateNameCheckDataIterator(DataIterator di, DataIteratorContext context, boolean isUpdateUsingLsid, TableInfo tableInfo) { @@ -3093,6 +3099,60 @@ protected DuplicateNameCheckDataIterator(DataIterator di, DataIteratorContext co Map map = DataIteratorUtil.createColumnNameMap(di); _nameCol = map.get(NAME_FIELD); _lsidCol = map.get("lsid"); + initCache(); + } + + private void initCache() + { + // if existing total row <= INIT_CACHE_LIMIT initial size, use cache + SQLFragment countSql = new SQLFragment("SELECT COUNT(*) FROM ").append(_tableInfo); + Long existingRows = new SqlSelector(ExperimentService.get().getSchema(), countSql).getObject(Long.class); + if (existingRows <= INIT_CACHE_LIMIT) + { + _useNameCache = true; + SQLFragment nameSql = new SQLFragment("SELECT DISTINCT(name) FROM ").append(_tableInfo); + String[] allExistingNames = new SqlSelector(ExperimentService.get().getSchema(), nameSql).getArray(String.class); + for (String existingName : allExistingNames) + { + newOrExistingNameLc.add(existingName.toLowerCase()); + if (_context.getInsertOption().mergeRows) + newOrExistingName.add(existingName); + } + } + } + + private boolean validateNameUsingCache(String newName) + { + if (!_useNameCache) + return false; + + String newNameLc = newName.toLowerCase(); + if (!_context.getInsertOption().allowUpdate) // insert doesn't allow case different match + { + if (newOrExistingNameLc.contains(newNameLc)) + addNameError(newName); + newOrExistingNameLc.add(newNameLc); + return true; + } + else if (_context.getInsertOption().mergeRows) + { + if (newOrExistingName.contains(newName)) // OK to contain exact match, this is an update + return true; + if (!newName.equals(newNameLc) && newOrExistingNameLc.contains(newNameLc)) // not ok to contain case different match + addNameError(newName); + newOrExistingNameLc.add(newNameLc); + return true; + } + + return false; // for update action, always use sql + } + + private void addNameError(String newName) + { + String error = String.format("The name '%s' already exists.", newName); + if (_context.getInsertOption().mergeRows) + error = String.format("The name '%s' could not be resolved. Please check the casing of the provided name.", newName); + _context.getErrors().addRowError(new ValidationException(error)); } @Override @@ -3116,6 +3176,9 @@ public boolean next() throws BatchValidationException if (StringUtils.isEmpty(newName)) return hasNext; + if (validateNameUsingCache(newName)) + return hasNext; + Map existingValues = getExistingRecord(); if (existingValues != null && !existingValues.isEmpty() && existingValues.get(NAME_FIELD).equals(newName)) return hasNext; @@ -3141,12 +3204,7 @@ else if (_context.getInsertOption().mergeRows) // merge } if (!isNameValid) - { - String error = String.format("The name '%s' already exists.", newName); - if (_context.getInsertOption().mergeRows) - error = String.format("The name '%s' could not be resolved. Please check the casing of the provided name.", newName); - _context.getErrors().addRowError(new ValidationException(error)); - } + addNameError(newName); return hasNext; } diff --git a/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java b/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java index fc87dd45df5..d85c9f03827 100644 --- a/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java +++ b/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java @@ -114,8 +114,10 @@ public class DataClassDomainKind extends AbstractDomainKind getRequiredIndices(Domain domain, for (PropertyStorageSpec.Index index : requiredIndices) { + if (sqlDialect.isSqlServer() && index.isCaseInsensitive) // skip case insensitive index for sql server + continue; + // TODO: Bad!! Shouldn't be making up an index name here! Ideally, we use the AbstractAuditTypeProvider.updateIndices() approach instead. - requiredIndicesMap.put(sqlDialect.nameIndex(storageTableName, index.columnNames), index); + requiredIndicesMap.put(sqlDialect.nameIndex(storageTableName, index), index); } return requiredIndicesMap; }