Issue 53567: Improve "Alias" column performance

api/src/org/labkey/api/audit/AuditHandler.java

@@ -87,7 +87,7 @@ static Pair<Map<String, Object>, Map<String, Object>> getOldAndNewRecordForMerge
             String key = entry.getKey();
             // getDatasetRows() (at least) should return key==column.getName(), expect getColumn(name) to work
             ColumnInfo col = null==table ? null : table.getColumn(key);
-            if (col != null && col.getFk() instanceof MultiValuedForeignKey)
+            if (col != null && (col.isMultiValued() || col.getFk() instanceof MultiValuedForeignKey))
                 isMultiValued = true;
 
             String nameFromAlias = key;
@@ -99,7 +99,7 @@ static Pair<Map<String, Object>, Map<String, Object>> getOldAndNewRecordForMerge
 
                 if (aliasColumn != null)
                 {
-                    if (aliasColumn.getFk() != null && aliasColumn.getFk() instanceof MultiValuedForeignKey)
+                    if (aliasColumn.getFk() != null && (aliasColumn.isMultiValued() || aliasColumn.getFk() instanceof MultiValuedForeignKey))
                         isMultiValued = true;
                     nameFromAlias = aliasColumn.getName();
                 }

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

@@ -842,4 +842,10 @@ public boolean isScannable()
     {
         return delegate.isScannable();
     }
+
+    @Override
+    public boolean isMultiValued()
+    {
+        return delegate.isMultiValued();
+    }
 }

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

@@ -97,6 +97,7 @@ public class BaseColumnInfo extends ColumnRenderPropertiesImpl implements Mutabl
     private String _jdbcDefaultValue = null;  // TODO: Merge with defaultValue, see #17646
     private boolean _isAutoIncrement = false;
     private boolean _hasDbSequence = false;
+    private boolean _isMultiValued = false;
     private boolean _isRootDbSequence = false;
     private boolean _isKeyField = false;
     private boolean _isReadOnly = false;
@@ -324,6 +325,7 @@ public void copyAttributesFrom(ColumnInfo col)
 
         // and the remaining
         setUserEditable(col.isUserEditable());
+        setIsMultiValued(col.isMultiValued());
         setNullable(col.isNullable());
         setRequired(col.isRequiredSet());
         setAutoIncrement(col.isAutoIncrement());
@@ -445,6 +447,7 @@ public void setExtraAttributesFrom(ColumnInfo col)
 
         setDerivationDataScope(col.getDerivationDataScope());
         setScannable(col.isScannable());
+        setIsMultiValued(col.isMultiValued());
     }
 
     /*
@@ -988,6 +991,17 @@ public void setIsRootDbSequence(boolean isRootDbSequence)
         _isRootDbSequence = isRootDbSequence;
     }
 
+    public boolean isMultiValued()
+    {
+        return _isMultiValued;
+    }
+
+    public void setIsMultiValued(boolean isMultiValued)
+    {
+        checkLocked();
+        _isMultiValued = isMultiValued;
+    }
+
     @Override
     public boolean isReadOnly()
     {

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

@@ -287,6 +287,8 @@ default boolean isSortable()
      */
     boolean isKeyField();
 
+    boolean isMultiValued();
+
     @Override
     boolean isMvEnabled();
 

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

@@ -211,4 +211,10 @@ protected SQLFragment getAggregateFunction(SQLFragment sql)
         // Can't sort because we need to make sure that all the multi-value columns come back in the same order
         return getSqlDialect().getGroupConcat(sql, false, false, new SQLFragment().appendStringLiteral(MultiValuedRenderContext.VALUE_DELIMITER, getSqlDialect()), true);
     }
+
+    @Override
+    public boolean isMultiValued()
+    {
+        return true;
+    }
 }

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

@@ -487,36 +487,37 @@ public SQLFragment getGroupConcat(SQLFragment sql, boolean distinct, boolean sor
     {
         // Sort function might not exist in external datasource; skip that syntax if not
         boolean useSortFunction = sorted && _arraySortFunctionExists.get();
+        SQLFragment result = new SQLFragment();
 
-        // TODO: Use "string_agg()" in place of array_to_string(array_agg())?
-        SQLFragment result = new SQLFragment("array_to_string(");
         if (useSortFunction)
         {
+            result.append("array_to_string(");
             result.append("core.sort(");   // TODO: Switch to use ORDER BY option inside array aggregate instead of our custom function
+            result.append("array_agg(");
         }
-        result.append("array_agg(");
-        if (distinct)
+        else
         {
-            result.append("DISTINCT ");
+            result.append("string_agg(");
         }
-        if (includeNulls)
+
+        if (distinct)
         {
-            result.append("COALESCE(CAST(");
+            result.append("DISTINCT ");
         }
         result.append(sql);
 
         if (includeNulls)
         {
-            result.append(" AS VARCHAR), '')");
+            result.append("::text");
         }
-        result.append(")");
         if (useSortFunction)
         {
-            result.append(")");
+            result.append(")"); // array_agg
+            result.append(")"); // core.sort
         }
         result.append(", ");
         result.append(delimiterSQL);
-        result.append(")");
+        result.append(")"); // array_to_string | string_agg
 
         return result;
     }

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

@@ -65,7 +65,7 @@ void init(TableInfo target, boolean useImportAliases, boolean outputAllColumns)
                 seen.add(to.getName());
                 if (to.getPropertyType() == PropertyType.ATTACHMENT || to.getPropertyType() == PropertyType.FILE_LINK)
                     addColumn(to, i);
-                else if (to.getFk() instanceof MultiValuedForeignKey)
+                else if (to.isMultiValued() || to.getFk() instanceof MultiValuedForeignKey)
                     addColumn(to.getName(), i); // pass-through multi-value columns -- converting will stringify a collection
                 else
                     addConvertColumn(to.getName(), i, to.getJdbcType(), to.getFk(), RemapMissingBehavior.Error, true);

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

@@ -1356,7 +1356,7 @@ private SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fro
             c = new RemappingConvertColumn(c, fromIndex, col, missing, true, lookupResolutionType);
         }
 
-        boolean multiValue = fk instanceof MultiValuedForeignKey;
+        boolean multiValue = col.isMultiValued() || fk instanceof MultiValuedForeignKey;
         if (multiValue)
         {
             // convert input into Collection of jdbcType values

api/src/org/labkey/api/query/AbstractQueryUpdateService.java

@@ -716,6 +716,39 @@ public List<Map<String, Object>> insertRows(User user, Container container, List
         return null;
     }
 
+    protected Object coerceTypesValue(ColumnInfo col, Map<String, Object> providedValues, String key, Object value)
+    {
+        if (col != null && value != null &&
+            !col.getJavaObjectClass().isInstance(value) &&
+            !(value instanceof AttachmentFile) &&
+            !(value instanceof MultipartFile) &&
+            !(value instanceof String[]) &&
+            !(col.isMultiValued() || col.getFk() instanceof MultiValuedForeignKey))
+        {
+            try
+            {
+                if (PropertyType.FILE_LINK.equals(col.getPropertyType()))
+                    value = ExpDataFileConverter.convert(value);
+                else if (col.getKindOfQuantity() != null)
+                {
+                    providedValues.put(key, value);
+                    value = Quantity.convert(value, col.getKindOfQuantity().getStorageUnit());
+                }
+                else
+                    value = col.getConvertFn().apply(value);
+            }
+            catch (ConvertHelper.FileConversionException e)
+            {
+                throw e;
+            }
+            catch (ConversionException e)
+            {
+                // That's OK, the transformation script may be able to fix up the value before it gets inserted
+            }
+        }
+
+        return value;
+    }
 
     /** Attempt to make the passed in types match the expected types so the script doesn't have to do the conversion */
     @Deprecated
@@ -726,42 +759,13 @@ protected Map<String, Object> coerceTypes(Map<String, Object> row, Map<String, O
         for (Map.Entry<String, Object> entry : row.entrySet())
         {
             ColumnInfo col = columnMap.get(entry.getKey());
-
-            Object value = entry.getValue();
-            if (col != null && value != null &&
-                    !col.getJavaObjectClass().isInstance(value) &&
-                    !(value instanceof AttachmentFile) &&
-                    !(value instanceof MultipartFile) &&
-                    !(value instanceof String[]) &&
-                    !(col.getFk() instanceof MultiValuedForeignKey))
-            {
-                try
-                {
-                    if (PropertyType.FILE_LINK.equals(col.getPropertyType()))
-                        value = ExpDataFileConverter.convert(value);
-                    else if (col.getKindOfQuantity() != null)
-                    {
-                        providedValues.put(entry.getKey(), value);
-                        value = Quantity.convert(value, col.getKindOfQuantity().getStorageUnit());
-                    }
-                    else
-                        value = col.getConvertFn().apply(value);
-                }
-                catch (ConvertHelper.FileConversionException e)
-                {
-                    throw e;
-                }
-                catch (ConversionException e)
-                {
-                    // That's OK, the transformation script may be able to fix up the value before it gets inserted
-                }
-            }
+            Object value = coerceTypesValue(col, providedValues, entry.getKey(), entry.getValue());
             result.put(entry.getKey(), value);
         }
+
         return result;
     }
 
-
     protected abstract Map<String, Object> updateRow(User user, Container container, Map<String, Object> row, @NotNull Map<String, Object> oldRow, @Nullable Map<Enum, Object> configParameters)
             throws InvalidKeyException, ValidationException, QueryUpdateServiceException, SQLException;
 

assay/package.json

@@ -12,7 +12,7 @@
     "clean": "rimraf resources/web/assay/gen && rimraf resources/views/gen && rimraf resources/web/gen"
   },
   "dependencies": {
-    "@labkey/components": "6.68.1"
+    "@labkey/components": "6.68.2-fb-alias-perf-53567.0"
   },
   "devDependencies": {
     "@labkey/build": "8.6.0",

core/package.json

@@ -53,7 +53,7 @@
     }
   },
   "dependencies": {
-    "@labkey/components": "6.68.1",
+    "@labkey/components": "6.68.2-fb-alias-perf-53567.0",
     "@labkey/themes": "1.4.2"
   },
   "devDependencies": {

experiment/package.json

@@ -13,7 +13,7 @@
     "test-integration": "cross-env NODE_ENV=test jest --ci --runInBand -c test/js/jest.config.integration.js"
   },
   "dependencies": {
-    "@labkey/components": "6.68.1"
+    "@labkey/components": "6.68.2-fb-alias-perf-53567.0"
   },
   "devDependencies": {
     "@labkey/build": "8.6.0",

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

@@ -6,43 +6,27 @@
 import org.labkey.api.data.DisplayColumnFactory;
 import org.labkey.api.data.MultiValuedDisplayColumn;
 import org.labkey.api.data.RenderContext;
-import org.labkey.api.data.TableSelector;
-import org.labkey.api.exp.api.ExperimentService;
+import org.labkey.api.util.PageFlowUtil;
 
-import java.util.Collections;
 import java.util.List;
 
 class AliasDisplayColumnFactory implements DisplayColumnFactory
 {
     @Override
     public DisplayColumn createRenderer(ColumnInfo colInfo)
     {
-        DataColumn dataColumn = new DataColumn(colInfo);
+        DataColumn dataColumn = new DataColumn(colInfo, false);
         dataColumn.setInputType("text");
 
-        return new MultiValuedDisplayColumn(dataColumn, true)
+        return new MultiValuedDisplayColumn(dataColumn)
         {
             @Override
             public Object getInputValue(RenderContext ctx)
             {
                 Object value = super.getInputValue(ctx);
-                StringBuilder sb = new StringBuilder();
                 if (value instanceof List)
-                {
-                    String delim = "";
-                    for (Object item : (List) value)
-                    {
-                        if (item != null)
-                        {
-                            String name = new TableSelector(ExperimentService.get().getTinfoAlias(), Collections.singleton("Name")).getObject(item, String.class);
-
-                            sb.append(delim);
-                            sb.append(name);
-                            delim = ", ";
-                        }
-                    }
-                }
-                return sb.toString();
+                    return PageFlowUtil.joinValuesToStringForExport((List<String>) value);
+                return "";
             }
         };
     }