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/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/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())
             {

core/src/org/labkey/core/CoreController.java

@@ -442,7 +442,8 @@ else if (form.getSchemaName() != null && form.getQueryName() != null && form.get
                     var select = QueryService.get().getSelectBuilder(table)
                             .columns(col)
                             .filter(filter);
-                    try (Results results = select.select())
+                    // Pass true for a cached result set so getSize() can report the row count without iterating
+                    try (Results results = select.select(true))
                     {
                         if (results.getSize() != 1 || !results.next())
                             throw new NotFoundException("Row not found for primary key");
@@ -859,7 +860,7 @@ public static class MoveContainerAction extends MutatingApiAction<SimpleApiJsonF
     {
         private Container target;
         private Container parent;
-        
+
         @Override
         public void validateForm(SimpleApiJsonForm form, Errors errors)
         {
@@ -882,7 +883,7 @@ public void validateForm(SimpleApiJsonForm form, Errors errors)
 
             // Worry about escaping
             Path path = Path.parse(targetIdentifier);
-            target = ContainerManager.getForPath(path);            
+            target = ContainerManager.getForPath(path);
 
             if (null == target)
             {
@@ -958,7 +959,7 @@ public ApiResponse execute(SimpleApiJsonForm form, BindException errors) throws
 
             List<String> aliasList = new ArrayList<>(ContainerManager.getAliasesForContainer(target));
             aliasList.add(target.getPath());
-            
+
             // Perform move
             ContainerManager.move(target, parent, getUser());
 
@@ -973,7 +974,7 @@ public ApiResponse execute(SimpleApiJsonForm form, BindException errors) throws
                 Map<String, Object> response = new HashMap<>();
                 response.put("success", true);
                 response.put("newPath", afterMoveTarget.getPath());
-                return new ApiSimpleResponse(response);                
+                return new ApiSimpleResponse(response);
             }
             return new ApiSimpleResponse();
         }
@@ -1124,7 +1125,7 @@ public void setMove(boolean move)
         {
             _move = move;
         }
-        
+
         public String getRequiredPermission()
         {
             return _requiredPermission;
@@ -1181,7 +1182,7 @@ public static class GetExtContainerTreeAction extends ReadOnlyApiAction<ExtConta
     {
         protected Class<? extends Permission> _reqPerm = ReadPermission.class;
         protected boolean _move = false;
-        
+
         @Override
         public ApiResponse execute(ExtContainerTreeForm form, BindException errors) throws Exception
         {
@@ -1323,7 +1324,7 @@ protected JSONObject getContainerProps(Container c, ExtContainerTreeForm form)
                 props.put("children", childrenProps);
                 props.put("expanded", true);
             }
-            
+
             return props;
         }
     }
@@ -1421,7 +1422,7 @@ private Map<String, Object> getContainerProps(Container c)
             return props;
         }
     }
-    
+
     public static class MoveWorkbookForm
     {
         public int _workbookId = -1;