Avoid calling getPivotValues() to improve PIVOT query performance

api/build.gradle

@@ -27,8 +27,6 @@ List apache = [
         "org.apache.commons:commons-lang3:${commonsLang3Version}",
         "commons-pool:commons-pool:${commonsPoolVersion}",
         "commons-validator:commons-validator:${commonsValidatorVersion}",
-        "org.apache.httpcomponents:httpclient:${httpclientVersion}",
-        "org.apache.httpcomponents:httpcore:${httpcoreVersion}",
         "org.apache.httpcomponents.client5:httpclient5:${httpclient5Version}",
         "org.apache.httpcomponents.core5:httpcore5:${httpcore5Version}",
         "org.apache.poi:poi:${poiVersion}",

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

@@ -129,7 +129,7 @@ abstract public class AbstractTableInfo implements TableInfo, AuditConfigurable,
     private int _cacheSize = CacheManager.DEFAULT_CACHE_SIZE;
 
     protected final Map<String, ColumnInfo> _columnMap;
-    /** Columns that aren't part of this table any more, but can still be resolved for backwards compatibility */
+    /** Columns that aren't part of this table anymore, but can still be resolved for backwards compatibility */
     protected final Map<String, ColumnInfo> _resolvedColumns = new CaseInsensitiveHashMap<>();
     private Map<String, MethodInfo> _methodMap;
     private Set<FieldKey> _methodFieldKeys;
@@ -162,6 +162,31 @@ abstract public class AbstractTableInfo implements TableInfo, AuditConfigurable,
 
     private final Map<String, CounterDefinition> _counterDefinitionMap = new CaseInsensitiveHashMap<>();    // Really only 1 for now, but could be more in future
 
+    private boolean _initialColumnsAreAdded = false;
+
+    protected boolean initialColumnsAreAdded()
+    {
+        return _initialColumnsAreAdded;
+    }
+
+    // Every method that interacts directly with _columnMap should call this first
+    private void ensureInitialColumnsAreAdded()
+    {
+        if (!_initialColumnsAreAdded)
+        {
+            _initialColumnsAreAdded = true;
+            initializeColumns();
+        }
+    }
+
+    // Initializing the column list can be expensive for certain TableInfos (e.g., PivotTableInfo). Adding all columns
+    // in an override of this method (instead of the constructor) allows the TableInfo to avoid that expensive operation
+    // in cases where the column list isn't needed (e.g., schema browser tree, query dependencies, linked schema
+    // drop-down list, etc.).
+    protected void initializeColumns()
+    {
+    }
+
     @NotNull
     @Override
     public List<ColumnInfo> getPkColumns()
@@ -217,7 +242,7 @@ public List<ColumnInfo> getAlternateKeyColumns()
     public AbstractTableInfo(DbSchema schema, String name)
     {
         _schema = schema;
-        _columnMap = constructColumnMap();
+        _columnMap = constructColumnMap(); // This is just an empty map, not populating any columns yet
         setName(name);
         addTriggerFactory(new ScriptTriggerFactory());
         MemTracker.getInstance().put(this);
@@ -499,6 +524,7 @@ public void setTitleColumn(String titleColumn, boolean defaultTitleColumn)
     @Nullable
     public ColumnInfo getColumn(@NotNull String name, boolean resolveIfNeeded)
     {
+        ensureInitialColumnsAreAdded();
         ColumnInfo ret = _columnMap.get(name);
         if (ret != null)
             return ret;
@@ -603,13 +629,15 @@ protected ColumnInfo resolveColumn(String name)
     @Override
     public List<ColumnInfo> getColumns()
     {
+        ensureInitialColumnsAreAdded();
         return List.copyOf(_columnMap.values());
     }
 
     @NotNull
     public List<MutableColumnInfo> getMutableColumns()
     {
         checkLocked();
+        ensureInitialColumnsAreAdded();
         return _columnMap.values().stream()
             .map(c -> (MutableColumnInfo)c)
             .peek(MutableColumnInfo::checkLocked)
@@ -619,6 +647,7 @@ public List<MutableColumnInfo> getMutableColumns()
     @Override
     public Set<String> getColumnNameSet()
     {
+        ensureInitialColumnsAreAdded();
         // Make the set case-insensitive
         return Collections.unmodifiableSet(new CaseInsensitiveTreeSet(_columnMap.keySet()));
     }
@@ -656,6 +685,7 @@ public void setDescription(String description)
     public boolean removeColumn(ColumnInfo column)
     {
         checkLocked();
+        ensureInitialColumnsAreAdded();
         // Clear the cached resolved columns so we regenerate it if the shape of the table changes
         _resolvedColumns.clear();
         return _columnMap.remove(column.getName()) != null;
@@ -664,6 +694,7 @@ public boolean removeColumn(ColumnInfo column)
     public MutableColumnInfo addColumn(MutableColumnInfo column)
     {
         checkLocked();
+        ensureInitialColumnsAreAdded();
         // Not true if this is a VirtualTableInfo
         // assert column.getParentTable() == this;
         if (_columnMap.containsKey(column.getName()))
@@ -694,6 +725,7 @@ public MutableColumnInfo addColumn(MutableColumnInfo column)
     public ColumnInfo replaceColumn(ColumnInfo updated, ColumnInfo existing)
     {
         checkLocked();
+        ensureInitialColumnsAreAdded();
         if (updated == existing)
             return updated;
 
@@ -1320,6 +1352,7 @@ protected void loadAllButCustomizerFromXML(QuerySchema schema, @Nullable TableTy
             {
                 // Reorder based on the sequence of columns in XML
                 Map<String, ColumnInfo> originalColumns = constructColumnMap();
+                assert initialColumnsAreAdded(); // getColumnNameSet() call above should have initialized the columns
                 originalColumns.putAll(_columnMap);
                 for (ColumnInfo columnInfo : originalColumns.values())
                 {
@@ -1337,7 +1370,7 @@ protected void loadAllButCustomizerFromXML(QuerySchema schema, @Nullable TableTy
                 }
                 for (ColumnInfo column : originalColumns.values())
                 {
-                    // Readd the rest of the columns that weren't in the XML. It's backed by a LinkedHashMap, so they'll
+                    // Read the rest of the columns that weren't in the XML. It's backed by a LinkedHashMap, so they'll
                     // be in the same order they were in originally
                     addColumn((BaseColumnInfo) column);
                 }

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

@@ -306,34 +306,16 @@ protected void addCrosstabQuery(SQLFragment sql)
         //finally group by the row dimensions
         sql.append("\nGROUP BY ");
         sep = "";
-        for(CrosstabDimension rowDim : getSettings().getRowAxis().getDimensions())
+        for (CrosstabDimension rowDim : getSettings().getRowAxis().getDimensions())
         {
-            sql.append(sep);
-            sql.append(PIVOT_ALIAS + "." + rowDim.getSourceColumn().getAlias());
+            sql.append(sep)
+                .append(PIVOT_ALIAS + ".")
+                .append(rowDim.getSourceColumn().getAlias());
             sep = ", ";
         }
 
     } //addCrosstabQuery()
 
-    public Map<String, String> getMeasureNameToColumnNameMap()
-    {
-        Map<String, String> measureNameToColumnName = new HashMap<>();
-        for (Map.Entry<String, ColumnInfo> entry : _columnMap.entrySet())
-        {
-            String colName = entry.getKey();
-            ColumnInfo col = entry.getValue();
-            String measureName;
-            if (col instanceof AggregateColumnInfo)
-                measureName = String.valueOf(((AggregateColumnInfo) col).getMember().getValue());
-            else if (col instanceof DimensionColumnInfo)
-                measureName = ((DimensionColumnInfo) col).getSourceFieldKey().toString();
-            else
-                measureName = col.getName();
-            measureNameToColumnName.put(measureName, colName);
-        }
-        return measureNameToColumnName;
-    }
-
     protected void addPivotQuery(SQLFragment sql)
     {
         sql.append("SELECT ");
@@ -342,8 +324,9 @@ protected void addPivotQuery(SQLFragment sql)
         String sep = "";
         for(CrosstabDimension rowDim : getSettings().getRowAxis().getDimensions())
         {
-            sql.append(sep);
-            sql.append(AGG_FILTERED_ALIAS + "." + rowDim.getSourceColumn().getAlias());
+            sql.append(sep)
+                .append(AGG_FILTERED_ALIAS + ".")
+                .append(rowDim.getSourceColumn().getAlias());
             sep = ", ";
         }
 
@@ -431,7 +414,7 @@ protected void addMemberCase(SQLFragment sql, CrosstabMember member, String quer
         CrosstabDimension colDimension = getSettings().getColumnAxis().getDimensions().get(0);
 
         sql.append("(CASE WHEN ");
-        sql.append(queryAlias + "." + colDimension.getSourceColumn().getAlias());
+        sql.append(queryAlias).append(".").append(colDimension.getSourceColumn().getAlias());
         sql.append("=");
         String value = getSQLValue(colDimension.getSourceColumn().getJdbcType(), member.getValue());
         sql.append(value);
@@ -530,19 +513,6 @@ public SQLFragment getFromSQL()
         return sql;
     }
 
-
-    /**
-     * Returns a column map suitable for passing to SimpleFilter's getSQLFragment() method.
-     * The map contains the same columns that are added when the table info is constructed
-     * without any column members (e.g., for customize view).
-     *
-     * @return A column map
-     */
-    protected Map<String, ColumnInfo> getAggregateFilterColMap()
-    {
-        return new CrosstabTable(getSettings())._columnMap;
-    }
-
     protected Map<FieldKey, ColumnInfo> getAggregateFilterColMap(Collection<ColumnInfo> cols)
     {
         Map<FieldKey,ColumnInfo> map = new HashMap<>(cols.size());

query/src/org/labkey/query/QueryDefinitionImpl.java

@@ -565,7 +565,7 @@ public TableInfo getTable(@NotNull UserSchema schema, @Nullable List<QueryExcept
                     table = createTable(schema, errors, includeMetadata, null, skipSuggestedColumns);
                 }
 
-                if (null == table)
+                if (null == table || (null != errors && !errors.isEmpty()))
                     return null;
 
                 log.debug("Caching table " + schema.getName() + "." + table.getName());