Issue 52657: sample name case sensitivity query performance

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

@@ -431,13 +431,15 @@ 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)
         {
             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,21 +453,31 @@ 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<String> 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)
         {
             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<String> piColumns = new CaseInsensitiveHashSet(Arrays.asList(propertyIndex.columnNames));
             Set<String> tiColumns = new CaseInsensitiveHashSet(Arrays.asList(tableIndex.columnNames));

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);

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

@@ -1182,8 +1182,8 @@ private List<String> 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<String> 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();

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");
     }
 

api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java

@@ -152,7 +152,8 @@ public class SampleTypeDomainKind extends AbstractDomainKind<SampleTypeDomainKin
         INDEXES = Collections.unmodifiableSet(Sets.newLinkedHashSet(Arrays.asList(
             new PropertyStorageSpec.Index(true, "rowId"),
             new PropertyStorageSpec.Index(true, "lsid"),
-            new PropertyStorageSpec.Index(true, "name")
+            new PropertyStorageSpec.Index(true, "name"),
+            new PropertyStorageSpec.Index(true, false, true, "name")
         )));
 
         FORCE_ENABLED_SYSTEM_FIELDS = Collections.unmodifiableSet(Sets.newHashSet(Arrays.asList("Name", "SampleState")));

experiment/src/org/labkey/experiment/ExpDataIterators.java

@@ -34,7 +34,9 @@
 import org.labkey.api.data.DbScope;
 import org.labkey.api.data.NameGenerator;
 import org.labkey.api.data.RemapCache;
+import org.labkey.api.data.SQLFragment;
 import org.labkey.api.data.SimpleFilter;
+import org.labkey.api.data.SqlSelector;
 import org.labkey.api.data.TSVWriter;
 import org.labkey.api.data.TableInfo;
 import org.labkey.api.data.TableSelector;
@@ -3078,11 +3080,15 @@ public DataIterator getDataIterator(DataIteratorContext context)
     public static class DuplicateNameCheckDataIterator extends WrapperDataIterator
     {
         final static String NAME_FIELD = "name";
+        final static Integer INIT_CACHE_LIMIT = 100_000;
         private final DataIteratorContext _context;
         private final Integer _nameCol;
         private final Integer _lsidCol;
         private final TableInfo _tableInfo;
         private final boolean _isUpdateUsingLsid;
+        private boolean _useNameCache = false;
+        private final Set<String> newOrExistingName = new HashSet<>(); // use lower case
+        private final Set<String> 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<String, Integer> 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<String, Object> 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;
         }

experiment/src/org/labkey/experiment/api/DataClassDomainKind.java

@@ -114,8 +114,10 @@ public class DataClassDomainKind extends AbstractDomainKind<DataClassDomainKindP
                 new PropertyStorageSpec.ForeignKey("lsid", "exp", "Data", "LSID", null, false)
         )));
 
-        INDEXES = Collections.unmodifiableSet(Sets.newLinkedHashSet(Arrays.asList(new PropertyStorageSpec.Index(true, "lsid"),
-                new PropertyStorageSpec.Index(true, "name", "classid"))));
+        INDEXES = Collections.unmodifiableSet(Sets.newLinkedHashSet(Arrays.asList(
+                new PropertyStorageSpec.Index(true, "lsid"),
+                new PropertyStorageSpec.Index(true, "name", "classid"),
+                new PropertyStorageSpec.Index(true, false, true, "name"))));
 
         FORCE_ENABLED_SYSTEM_FIELDS = Collections.unmodifiableSet(Sets.newHashSet(Arrays.asList("Name")));
     }

experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java

@@ -899,8 +899,11 @@ private Map<String, PropertyStorageSpec.Index> 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;
     }