Disable LabKey caching of ResultSet by default

api/src/org/labkey/api/assay/AssayTableMetadata.java

@@ -223,7 +223,7 @@ public Collection<String> getTargetStudyContainers(AssaySchema schema, TableInfo
             tableMap.put("__TARGET_STUDY_TABLE", targetStudyTable);
             tableMap.put("__DATA", assayDataTable);
 
-            try (ResultSet rs = QueryService.get().select(schema, sqlf.getSQL(), tableMap, false, true))
+            try (ResultSet rs = QueryService.get().getSelectBuilder(schema, sqlf.getSQL(), false, tableMap).select(true))
             {
                 ArrayList<String> ids = new ArrayList<>();
                 while (rs.next())

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

@@ -21,6 +21,7 @@
 import org.labkey.api.dataiterator.DataIteratorUtil;
 import org.labkey.api.query.BatchValidationException;
 import org.labkey.api.query.FieldKey;
+import org.labkey.api.view.UnauthorizedException;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -141,8 +142,7 @@ public boolean isComplete()
     @Override
     public Map<String, Object> getRowMap()
     {
-        // TODO
-        return null;
+        throw new UnauthorizedException();
     }
 
     @Override
@@ -154,8 +154,7 @@ public Map<String, Object> getRowMap()
     @Override
     public @NotNull Iterator<Map<String, Object>> iterator()
     {
-        // TODO
-        return null;
+        throw new UnauthorizedException();
     }
 
     @Override
@@ -362,8 +361,7 @@ public void close()
     @Override
     public boolean wasNull()
     {
-        // TODO
-        return false;
+        throw new UnauthorizedException();
     }
 
     @Override

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

@@ -67,6 +67,89 @@ protected void verifyResultSets(SqlSelector sqlSelector, int expectedRowCount, b
         verifyResultSet(sqlSelector.getResultSet(false, false), expectedRowCount, expectedComplete);
         verifyResultSet(sqlSelector.getResultSet(false, true), expectedRowCount, expectedComplete);
         verifyResultSet(sqlSelector.getResultSet(true, true), expectedRowCount, expectedComplete);
+
+        // Verify getSize() and backward-scrolling behavior for each caching/scrollable combination. These behaviors
+        // differ between a cached result set (CachedResultSet, cache == true) and a non-cached result set
+        // (ResultSetImpl, cache == false), and silently switching a caller from cached to non-cached has caused
+        // regressions (getSize() called before iteration, or beforeFirst() used to re-iterate).
+        verifyCachedResultSet(sqlSelector, expectedRowCount);
+        verifyForwardOnlyResultSet(sqlSelector, expectedRowCount);
+        verifyScrollableUncachedResultSet(sqlSelector, expectedRowCount);
+    }
+
+    // A cached result set (getResultSet(true, true) -> CachedResultSet) knows its size without iterating and supports
+    // backward scrolling, so it can be re-iterated after beforeFirst().
+    private void verifyCachedResultSet(SqlSelector selector, int expectedRowCount) throws SQLException
+    {
+        try (TableResultSet rs = selector.getResultSet(true, true))
+        {
+            // getSize() must work before any iteration
+            assertEquals("Cached ResultSet should report its size before iteration", expectedRowCount, rs.getSize());
+
+            int count = 0;
+            while (rs.next())
+                count++;
+            assertEquals(expectedRowCount, count);
+
+            // Scroll back to the start and re-iterate
+            rs.beforeFirst();
+            int recount = 0;
+            while (rs.next())
+                recount++;
+            assertEquals("Cached ResultSet should be re-iterable after beforeFirst()", expectedRowCount, recount);
+        }
+    }
+
+    // A non-cached, forward-only result set (getResultSet(false, false) -> ResultSetImpl) cannot report its size until
+    // it has been completely iterated, and cannot scroll backward.
+    private void verifyForwardOnlyResultSet(SqlSelector selector, int expectedRowCount) throws SQLException
+    {
+        // getSize() throws until the result set has been completely iterated
+        try (TableResultSet rs = selector.getResultSet(false, false))
+        {
+            assertThrows("getSize() should throw before a non-cached ResultSet is fully iterated", IllegalStateException.class, rs::getSize);
+        }
+
+        // Backward scrolling is not supported on a forward-only result set
+        try (TableResultSet rs = selector.getResultSet(false, false))
+        {
+            assertThrows("beforeFirst() should throw on a forward-only ResultSet", SQLException.class, rs::beforeFirst);
+        }
+
+        // After complete iteration getSize() reports the row count
+        try (TableResultSet rs = selector.getResultSet(false, false))
+        {
+            int count = 0;
+            while (rs.next())
+                count++;
+            assertEquals(expectedRowCount, count);
+            assertEquals("getSize() should report the row count after complete iteration", expectedRowCount, rs.getSize());
+        }
+    }
+
+    // A non-cached but scrollable result set (getResultSet(false, true) -> ResultSetImpl over a scrollable JDBC
+    // ResultSet) supports backward scrolling, but still cannot report its size until completely iterated because
+    // getSize() depends on caching, not scrollability.
+    private void verifyScrollableUncachedResultSet(SqlSelector selector, int expectedRowCount) throws SQLException
+    {
+        try (TableResultSet rs = selector.getResultSet(false, true))
+        {
+            int count = 0;
+            while (rs.next())
+                count++;
+            assertEquals(expectedRowCount, count);
+
+            rs.beforeFirst();
+            int recount = 0;
+            while (rs.next())
+                recount++;
+            assertEquals("Scrollable ResultSet should be re-iterable after beforeFirst()", expectedRowCount, recount);
+        }
+
+        try (TableResultSet rs = selector.getResultSet(false, true))
+        {
+            assertThrows("getSize() should throw before a non-cached ResultSet is fully iterated", IllegalStateException.class, rs::getSize);
+        }
     }
 
     public static class TestClass

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

@@ -28,6 +28,9 @@ public interface TableResultSet extends ResultSet, Iterable<Map<String, Object>>
 {
     boolean isComplete();
 
+    /**
+     * Only supported by CachedResultSet. Other implementations will throw UnsupportedOperationException
+     */
     Map<String, Object> getRowMap() throws SQLException;
 
     @Override

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

@@ -215,7 +215,9 @@ private SimpleFilter getViewFilter(CustomView baseView)
                 .columns(cols)
                 .filter(filter)
                 .sort(sort);
-        return select.select();
+        // select(true): legacy callers (especially EHR modules) rely on the cached result set's full API,
+        // e.g. getRowMap(), which a streaming result set does not support
+        return select.select(true);
     }
 
     public Results select(SimpleFilter filter)

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

@@ -239,36 +239,12 @@ UserSchema createLinkedSchema(User user, Container container, String name, Strin
 
     void saveCalculatedFieldsMetadata(String schemaName, String queryName, @Nullable String updatedQueryName, List<? extends GWTPropertyDescriptor> fields, boolean hasExistingFields, User user, Container container) throws MetadataUnavailableException;
 
-    /**
-     * Create a TableSelector for a LabKey sql query string.
-     * @param schema The query schema context used to parse the sql query in.
-     * @param sql The LabKey query string.
-     * @return a TableSelector
-     *
-     * superseded by {@link QueryService#getSelectBuilder}
-     */
-    @NotNull
-    TableSelector selector(@NotNull QuerySchema schema, @NotNull String sql);
-
-    /** superseded by {@link QueryService#getSelectBuilder}  */
-	default ResultSet select(QuerySchema schema, String sql)
-    {
-        return select(schema, sql, null, false, true);
-    }
-
-    /* strictColumnList requires that query not add any addition columns to the query result */
-    Results select(QuerySchema schema, String sql, @Nullable Map<String, TableInfo> tableMap, boolean strictColumnList, boolean cached);
-
-    /** superseded by {@link QueryService#getSelectBuilder}  */
-    default Results select(TableInfo table, Collection<ColumnInfo> columns, @Nullable Filter filter, @Nullable Sort sort)
-    {
-        return getSelectBuilder(table).columns(columns).filter(filter).sort(sort).select();
-    }
-
     SelectBuilder getSelectBuilder(TableInfo table);
     SelectBuilder getSelectBuilder(QuerySchema schema, String sql);
     /** Use when the query must not return extra hidden sort columns (e.g. HTTP endpoints that reflect column names to callers). */
     SelectBuilder getSelectBuilder(QuerySchema schema, String sql, boolean strictColumnList);
+    /** The tableMap binds named table references in the SQL (e.g. "__DATA") to TableInfos. It must be supplied here before the SQL is parsed. */
+    SelectBuilder getSelectBuilder(QuerySchema schema, String sql, boolean strictColumnList, @Nullable Map<String, TableInfo> tableMap);
 
     MutableColumnInfo createQueryExpressionColumn(TableInfo table, FieldKey key, String labKeySql, ColumnType columnType);
     MutableColumnInfo createQueryExpressionColumn(TableInfo table, FieldKey key, FieldKey wrapped, ColumnType columnType);
@@ -279,7 +255,7 @@ default Results select(TableInfo table, Collection<ColumnInfo> columns, @Nullabl
     Collection<CompareType> getCompareTypes();
 
     /**
-     * Gets all of the custom views from the given relative path defined in the set of active modules for the
+     * Gets all the custom views from the given relative path defined in the set of active modules for the
      * given container.
      * @param container Container to use to figure out the set of active modules
      * @param qd the query for which views should be fetched
@@ -692,15 +668,19 @@ default SelectBuilder columns(ColumnInfo... cols)
         SelectBuilder jdbcCaching(boolean jdbcCaching);
 
         SQLFragment buildSqlFragment();
-        SqlSelector buildSqlSelector();
+        default SqlSelector buildSqlSelector()
+        {
+            return buildSqlSelector(Map.of());
+        }
+        SqlSelector buildSqlSelector(@NotNull Map<String, Object> parameters);
         Results select(boolean labkeyCachedResultSet, @NotNull Map<String, Object> parameters);
         default Results select(boolean labkeyCachedResultSet)
         {
             return select(labkeyCachedResultSet, Map.of());
         }
         default Results select()
         {
-            return select(true);
+            return select(false);
         }
 
         QueryLogging getQueryLogging();

api/src/org/labkey/api/visualization/VisDataRequest.java

@@ -242,7 +242,7 @@ public static class Measure
         String axisName;
         Boolean requireLeftJoin;
         String schemaName;
-        List<Object> values = new ArrayList<>();
+        List<?> values = new ArrayList<>();
 
         public Measure()
         {}
@@ -410,12 +410,12 @@ public Measure setNsvalues(String nsvalues)
             return this;
         }
 
-        public List<Object> getValues()
+        public List<?> getValues()
         {
             return values;
         }
 
-        public Measure setValues(List<Object> values)
+        public Measure setValues(List<?> values)
         {
             this.values = values;
             return this;

api/src/org/labkey/api/visualization/VisualizationSourceColumn.java

@@ -167,7 +167,7 @@ protected VisualizationSourceColumn(ViewContext context, VisDataRequest.Measure
         if (StringUtils.isNotEmpty(measure.getAxisName()))
             _axisName = measure.getAxisName();
 
-        List<Object> values = measure.getValues();
+        List<?> values = measure.getValues();
         _clientAlias = measure.getAlias();
         if (values != null)
         {

assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java

@@ -431,7 +431,7 @@ public DataIteratorBuilder mergeReRunData(
         try
         {
             SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Run"), run.getRowId());
-            try (Results results = QueryService.get().select(resultsTable, columnInfoMap.values(), filter, new Sort(FieldKey.fromParts("RowId"))))
+            try (Results results = QueryService.get().getSelectBuilder(resultsTable).columns(columnInfoMap.values()).filter(filter).sort(new Sort(FieldKey.fromParts("RowId"))).select())
             {
                 while (results.next())
                 {

assay/src/org/labkey/assay/plate/PlateManager.java

@@ -1699,7 +1699,7 @@ public PlateLayoutHandler getPlateLayoutHandler(String plateTypeName)
     {
         return _plateLayoutHandlers.get(plateTypeName);
     }
-    
+
     public UserSchema getPlateUserSchema(Container container, User user)
     {
         return QueryService.get().getUserSchema(user, container, PlateSchema.SCHEMA_NAME);
@@ -1837,7 +1837,7 @@ public boolean hasPermission(Lsid lsid, @NotNull User user, @NotNull Class<? ext
             return false;
         }
     }
-    
+
     public void registerLsidHandlers()
     {
         if (_lsidHandlersRegistered)
@@ -2662,7 +2662,7 @@ else if (f2.isBuiltIn())
 
         TableInfo wellTable = getWellTable(plate.getContainer(), user);
         Map<FieldKey, ColumnInfo> columnMap = QueryService.get().getColumns(wellTable, customFieldMap.keySet());
-        try (Results r = QueryService.get().select(wellTable, columnMap.values(), filter, null))
+        try (Results r = QueryService.get().getSelectBuilder(wellTable).columns(columnMap.values()).filter(filter).select())
         {
             while (r.next())
             {
@@ -4296,7 +4296,8 @@ private void validatePlateSetReplicateGroups(Container container, User user, @No
             return;
 
         var sql = getReplicateGroupLabKeySql(plateSchema, plateSetRowId);
-        try (var results = QueryService.get().getSelectBuilder(plateSchema, sql).select())
+        // Pass true for a cached result set so getSize() can report the row count without iterating
+        try (var results = QueryService.get().getSelectBuilder(plateSchema, sql).select(true))
         {
             if (replicateWellGroupCount == results.getSize())
                 return;
@@ -4361,7 +4362,8 @@ private void validatePlateSetSampleGroups(Container container, User user, @NotNu
             return;
 
         var sampleGroupLabKeySql = getSampleGroupLabKeySql(plateSetRowId, true);
-        try (var results = QueryService.get().getSelectBuilder(plateSchema, sampleGroupLabKeySql).select())
+        // Pass true for a cached result set so getSize() can report the row count without iterating
+        try (var results = QueryService.get().getSelectBuilder(plateSchema, sampleGroupLabKeySql).select(true))
         {
             if (sampleGroupCount == results.getSize())
                 return;

assay/src/org/labkey/assay/plate/PlateManagerTest.java

@@ -484,7 +484,7 @@ public void testCreatePlateMetadata() throws Exception
         Map<FieldKey, ColumnInfo> columns = QueryService.get().getColumns(wellTable, List.of(fkConcentration, fkNegativeControl));
 
         // verify plate metadata property updates
-        try (Results r = QueryService.get().select(wellTable, columns.values(), filter, new Sort("Col")))
+        try (Results r = QueryService.get().getSelectBuilder(wellTable).columns(columns.values()).filter(filter).sort(new Sort("Col")).select())
         {
             int row = 0;
             while (r.next())
@@ -531,7 +531,7 @@ public void testCreateAndSavePlateWithData() throws Exception
         SimpleFilter filter = SimpleFilter.createContainerFilter(container);
         filter.addCondition(FieldKey.fromParts("PlateId"), plate.getRowId());
         filter.addCondition(FieldKey.fromParts("Row"), 0);
-        try (Results r = QueryService.get().select(wellTable, columns.values(), filter, new Sort("Col")))
+        try (Results r = QueryService.get().getSelectBuilder(wellTable).columns(columns.values()).filter(filter).sort(new Sort("Col")).select())
         {
             int row = 0;
             while (r.next())
@@ -2098,7 +2098,7 @@ private Results getPlateWellResults(long plateRowId)
         filter.addCondition(FieldKey.fromParts("PlateId"), plateRowId);
 
         var wellTable = getWellTable();
-        return QueryService.get().select(wellTable, getWellTableColumns(wellTable).values(), filter, new Sort("RowId"));
+        return QueryService.get().getSelectBuilder(wellTable).columns(getWellTableColumns(wellTable).values()).filter(filter).sort(new Sort("RowId")).select();
     }
 
     private Map<String, Object> getWellRow(long plateRowId, @NotNull String position)

assay/src/org/labkey/assay/plate/PlateSetExport.java

@@ -121,7 +121,9 @@ public static List<ColumnDescriptor> getColumnDescriptors(String prefix, List<Fi
     private Map<String, List<Object[]>> getSampleIdToRows(TableInfo wellTable, List<FieldKey> includedMetadataCols, long plateSetId, String plateSetExport)
     {
         Map<String, List<Object[]>> sampleIdToRow = new LinkedHashMap<>();
-        try (Results rs = QueryService.get().select(wellTable, getWellColumns(wellTable, includedMetadataCols), new SimpleFilter(FKMap.get(PLATE_SET_ID_COL), plateSetId), new Sort(ROW_ID_COL)))
+        // select(true): getDataRow() relies on the cached result set's column-type coercion (e.g. a numeric metadata
+        // value rendered as "1.0" rather than "1"); a streaming result set would change the exported value formatting
+        try (Results rs = QueryService.get().getSelectBuilder(wellTable).columns(getWellColumns(wellTable, includedMetadataCols)).filter(new SimpleFilter(FKMap.get(PLATE_SET_ID_COL), plateSetId)).sort(new Sort(ROW_ID_COL)).select(true))
         {
             while (rs.next())
             {
@@ -195,7 +197,9 @@ else if (sourceRows.size() != destinationRows.size())
     public List<Object[]> getInstrumentInstructions(TableInfo wellTable, long plateSetId, List<FieldKey> includedMetadataCols)
     {
         List<Object[]> plateDataRows = new ArrayList<>();
-        try (Results rs = QueryService.get().select(wellTable, getWellColumns(wellTable, includedMetadataCols), new SimpleFilter(FKMap.get(PLATE_SET_ID_COL), plateSetId), new Sort(ROW_ID_COL)))
+        // select(true): getDataRow() relies on the cached result set's column-type coercion (e.g. a numeric metadata
+        // value rendered as "1.0" rather than "1"); a streaming result set would change the exported value formatting
+        try (Results rs = QueryService.get().getSelectBuilder(wellTable).columns(getWellColumns(wellTable, includedMetadataCols)).filter(new SimpleFilter(FKMap.get(PLATE_SET_ID_COL), plateSetId)).sort(new Sort(ROW_ID_COL)).select(true))
         {
             while (rs.next())
             {