diff --git a/api/src/org/labkey/api/assay/AssayTableMetadata.java b/api/src/org/labkey/api/assay/AssayTableMetadata.java index 11aa3f889ef..89a59af4465 100644 --- a/api/src/org/labkey/api/assay/AssayTableMetadata.java +++ b/api/src/org/labkey/api/assay/AssayTableMetadata.java @@ -223,7 +223,7 @@ public Collection 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 ids = new ArrayList<>(); while (rs.next()) diff --git a/api/src/org/labkey/api/data/SqlSelectorTestCase.java b/api/src/org/labkey/api/data/SqlSelectorTestCase.java index e61251b3389..24245672849 100644 --- a/api/src/org/labkey/api/data/SqlSelectorTestCase.java +++ b/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 diff --git a/api/src/org/labkey/api/query/QueryService.java b/api/src/org/labkey/api/query/QueryService.java index 5faffb42feb..dc14c04d745 100644 --- a/api/src/org/labkey/api/query/QueryService.java +++ b/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 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 tableMap, boolean strictColumnList, boolean cached); - - /** superseded by {@link QueryService#getSelectBuilder} */ - default Results select(TableInfo table, Collection 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 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 columns, @Nullabl Collection 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,7 +668,11 @@ default SelectBuilder columns(ColumnInfo... cols) SelectBuilder jdbcCaching(boolean jdbcCaching); SQLFragment buildSqlFragment(); - SqlSelector buildSqlSelector(); + default SqlSelector buildSqlSelector() + { + return buildSqlSelector(Map.of()); + } + SqlSelector buildSqlSelector(@NotNull Map parameters); Results select(boolean labkeyCachedResultSet, @NotNull Map parameters); default Results select(boolean labkeyCachedResultSet) { @@ -700,7 +680,7 @@ default Results select(boolean labkeyCachedResultSet) } default Results select() { - return select(true); + return select(false); } QueryLogging getQueryLogging(); diff --git a/api/src/org/labkey/api/visualization/VisDataRequest.java b/api/src/org/labkey/api/visualization/VisDataRequest.java index fc18a4840d8..9a270e15a0d 100644 --- a/api/src/org/labkey/api/visualization/VisDataRequest.java +++ b/api/src/org/labkey/api/visualization/VisDataRequest.java @@ -242,7 +242,7 @@ public static class Measure String axisName; Boolean requireLeftJoin; String schemaName; - List values = new ArrayList<>(); + List values = new ArrayList<>(); public Measure() {} @@ -410,12 +410,12 @@ public Measure setNsvalues(String nsvalues) return this; } - public List getValues() + public List getValues() { return values; } - public Measure setValues(List values) + public Measure setValues(List values) { this.values = values; return this; diff --git a/api/src/org/labkey/api/visualization/VisualizationSourceColumn.java b/api/src/org/labkey/api/visualization/VisualizationSourceColumn.java index 5e3168036bb..6defadfc04a 100644 --- a/api/src/org/labkey/api/visualization/VisualizationSourceColumn.java +++ b/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 values = measure.getValues(); + List values = measure.getValues(); _clientAlias = measure.getAlias(); if (values != null) { diff --git a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java index 35df0589e9f..b7b2767e4f8 100644 --- a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java +++ b/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()) { diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index a7b6e7eef4a..a520b3f9fc0 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/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 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; diff --git a/assay/src/org/labkey/assay/plate/PlateManagerTest.java b/assay/src/org/labkey/assay/plate/PlateManagerTest.java index 5c2e2f4be9a..0846613c2c8 100644 --- a/assay/src/org/labkey/assay/plate/PlateManagerTest.java +++ b/assay/src/org/labkey/assay/plate/PlateManagerTest.java @@ -484,7 +484,7 @@ public void testCreatePlateMetadata() throws Exception Map 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 getWellRow(long plateRowId, @NotNull String position) diff --git a/assay/src/org/labkey/assay/plate/PlateSetExport.java b/assay/src/org/labkey/assay/plate/PlateSetExport.java index c60a4e0a5ac..69b1c8e57d3 100644 --- a/assay/src/org/labkey/assay/plate/PlateSetExport.java +++ b/assay/src/org/labkey/assay/plate/PlateSetExport.java @@ -121,7 +121,9 @@ public static List getColumnDescriptors(String prefix, List> getSampleIdToRows(TableInfo wellTable, List includedMetadataCols, long plateSetId, String plateSetExport) { Map> 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 getInstrumentInstructions(TableInfo wellTable, long plateSetId, List includedMetadataCols) { List 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()) { diff --git a/core/src/org/labkey/core/CoreController.java b/core/src/org/labkey/core/CoreController.java index ccdbe48e6fb..0fcd3d90c12 100644 --- a/core/src/org/labkey/core/CoreController.java +++ b/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 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 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 _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 getContainerProps(Container c) return props; } } - + public static class MoveWorkbookForm { public int _workbookId = -1; diff --git a/experiment/src/org/labkey/experiment/api/LineageTest.java b/experiment/src/org/labkey/experiment/api/LineageTest.java index f64b220bbd4..238c2d407a1 100644 --- a/experiment/src/org/labkey/experiment/api/LineageTest.java +++ b/experiment/src/org/labkey/experiment/api/LineageTest.java @@ -258,7 +258,7 @@ public void testDeriveDuringImport() throws Exception "FROM exp.data." + firstDataClassName + " AS dc\n" + "ORDER BY dc.RowId\n"; - try (Results rs = QueryService.get().getSelectBuilder(schema, sql, true).select()) + try (Results rs = QueryService.get().getSelectBuilder(schema, sql, true).select(true)) { RenderContext ctx = new RenderContext(new ViewContext()); ctx.getViewContext().setRequest(TestContext.get().getRequest()); @@ -501,12 +501,12 @@ public void testListAndSampleLineage() throws Exception throw errors; // query - TableSelector ts = QueryService.get().selector(listSchema, + QueryService.SelectBuilder builder = QueryService.get().getSelectBuilder(listSchema, "SELECT SampleId, SampleId.Inputs.Materials.MySamples.Name As MySampleParent FROM MyList"); RenderContext ctx; DisplayColumn dcSampleId; DisplayColumn dcMySampleParent; - try (Results results = ts.getResults()) + try (Results results = builder.select(true)) { ctx = new RenderContext(new ViewContext()); ctx.getViewContext().setRequest(TestContext.get().getRequest()); diff --git a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderWriter.java b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderWriter.java index f3d00361cc6..afc21e2cb53 100644 --- a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderWriter.java +++ b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderWriter.java @@ -107,7 +107,7 @@ public void initialize(FolderExportContext ctx) protected void writeTsv(TableInfo tinfo, Collection columns, SimpleFilter filter, Sort sort, VirtualFile dir, String baseName) throws IOException { - ResultsFactory factory = ()->QueryService.get().select(tinfo, columns, filter, sort); + ResultsFactory factory = ()->QueryService.get().getSelectBuilder(tinfo).columns(columns).filter(filter).sort(sort).select(); try (TSVGridWriter tsvWriter = new TSVGridWriter(factory)) { tsvWriter.setApplyFormats(false); @@ -268,7 +268,7 @@ protected void writeAttachments(Container c, TableInfo tinfo, VirtualFile dir) t for (ColumnInfo col : attachmentCols) uniquifiers.put(col.getName(), new FileNameUniquifier()); - try (ResultSet rs = QueryService.get().select(tinfo, selectColumns, null, null)) + try (ResultSet rs = QueryService.get().getSelectBuilder(tinfo).columns(selectColumns).select()) { while (rs.next()) { diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 42b9c18d478..1f80ac884e6 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -1571,39 +1571,22 @@ public List getQuerySnapshotDefs(Container container, S .collect(Collectors.toList()); } - private static class ContainerSchemaKey implements Serializable - { - @NotNull - private final Container _container; - @NotNull - private final SchemaKey _schema; - - public ContainerSchemaKey(@NotNull Container container, @NotNull SchemaKey schema) + private record ContainerSchemaKey(@NotNull Container _container, @NotNull SchemaKey _schema) implements Serializable { - _container = container; - _schema = schema; - } - @Override - public boolean equals(Object o) - { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; - ContainerSchemaKey that = (ContainerSchemaKey) o; + ContainerSchemaKey that = (ContainerSchemaKey) o; - if (!_container.equals(that._container)) return false; - return _schema.equals(that._schema); - } + if (!_container.equals(that._container)) return false; + return _schema.equals(that._schema); + } - @Override - public int hashCode() - { - int result = _container.hashCode(); - result = 31 * result + _schema.hashCode(); - return result; } - } @Override public QueryDefinition saveSessionQuery(ViewContext context, Container container, String schemaName, String sql, String metadataXml) @@ -1644,43 +1627,26 @@ public QueryDefinition saveSessionQuery(ViewContext context, Container container private static final String PERSISTED_TEMP_QUERIES_KEY = "LABKEY.PERSISTED_TEMP_QUERIES"; - private static class SessionQuery implements Serializable - { - private final String _sql; - private final String _metadata; - - public SessionQuery(String sql, String metadata) - { - _sql = sql; - _metadata = metadata; - } - - @Override - public int hashCode() + private record SessionQuery(String _sql, String _metadata) implements Serializable { - int result = _sql.hashCode(); - if (_metadata != null) - result = 31 * result + _metadata.hashCode(); - return result; - } - @Override - public boolean equals(Object obj) - { - if (obj instanceof SessionQuery sq) + @Override + public boolean equals(Object obj) { - if (!_sql.equals(sq._sql)) - return false; - if (_metadata == null && sq._metadata != null) - return false; - if (_metadata != null && !_metadata.equals(sq._metadata)) - return false; - - return true; + if (obj instanceof SessionQuery(String sql, String metadata)) + { + if (!_sql.equals(sql)) + return false; + if (_metadata == null && metadata != null) + return false; + if (_metadata != null && !_metadata.equals(metadata)) + return false; + + return true; + } + return false; } - return false; } - } private Map getSessionQueryMap(@NotNull HttpSession session, Container container, SchemaKey schemaName) { @@ -2460,46 +2426,35 @@ public void saveCalculatedFieldsMetadata(String schemaName, String queryName, @N // that the scope of the cache is very limited, and this is a very conservative cache. private final Map>>> _metadataCache = Collections.synchronizedMap(new WeakHashMap<>()); - /** Hides whatever the underlying key might do for .equals() and .hashCode() and instead relies on pointer equality */ - private static class ObjectIdentityCacheKey - { - private final Object _object; - private final boolean _customQuery; - private final boolean _allModules; - private final Path _dir; - - private ObjectIdentityCacheKey(UserSchema object, boolean customQuery, boolean allModules, @Nullable Path dir) - { - _object = object; - _customQuery = customQuery; - _allModules = allModules; - _dir = dir; - } - - @Override - public boolean equals(Object o) + /** + * Hides whatever the underlying key might do for .equals() and .hashCode() and instead relies on pointer equality + */ + private record ObjectIdentityCacheKey(Object _object, boolean _customQuery, boolean _allModules, Path _dir) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; - ObjectIdentityCacheKey that = (ObjectIdentityCacheKey) o; + ObjectIdentityCacheKey that = (ObjectIdentityCacheKey) o; - if (_allModules != that._allModules) return false; - if (_customQuery != that._customQuery) return false; - if (_dir != null ? !_dir.equals(that._dir) : that._dir != null) return false; - return that._object == _object; - } + if (_allModules != that._allModules) return false; + if (_customQuery != that._customQuery) return false; + if (!Objects.equals(_dir, that._dir)) return false; + return that._object == _object; + } - @Override - public int hashCode() - { - int result = System.identityHashCode(_object); - result = 31 * result + (_customQuery ? 1 : 0); - result = 31 * result + (_allModules ? 1 : 0); - result = 31 * result + (_dir != null ? _dir.hashCode() : 0); - return result; + @Override + public int hashCode() + { + int result = System.identityHashCode(_object); + result = 31 * result + (_customQuery ? 1 : 0); + result = 31 * result + (_allModules ? 1 : 0); + result = 31 * result + (_dir != null ? _dir.hashCode() : 0); + return result; + } } - } /** * Finds metadata overrides for the given schema and table and returns them in application order. @@ -2692,27 +2647,6 @@ public void registerMethod(String name, MethodInfo info, JdbcType returnType, in Method.addMethod(name, info, returnType, minArgs, maxArgs); } - @Override - @NotNull - public TableSelector selector(@NotNull QuerySchema schema, @NotNull String sql) - { - return new LabKeyQuerySelector(schema, sql); - } - - private static class LabKeyQuerySelector extends TableSelector - { - public LabKeyQuerySelector(@NotNull QuerySchema schema, @NotNull String sql) - { - super(QueryServiceImpl.get().getSelectBuilder(schema, sql).getTableInfo()); - } - - public LabKeyQuerySelector(@NotNull QuerySchema schema, @NotNull String sql, Set columnNames, @Nullable Filter filter, @Nullable Sort sort) - { - super(QueryServiceImpl.get().getSelectBuilder(schema, sql).getTableInfo(), columnNames, filter, sort); - } - } - - @Override public TableInfo createTable(QuerySchema schema, String sql, @Nullable Map tableMap, boolean strictColumnList) { @@ -2724,19 +2658,6 @@ public TableInfo createTable(QuerySchema schema, String sql, @Nullable Map tableMap, boolean strictColumnList, boolean cached) - { - Query q = new Query(schema); - q.setStrictColumnList(strictColumnList); - q.setTableMap(tableMap); - q.parse(sql); - - return getSelectBuilder(q).select(cached); - } - - @Override public void bindNamedParameters(SQLFragment frag, @Nullable Map in) { @@ -2816,10 +2737,16 @@ public SelectBuilder getSelectBuilder(QuerySchema schema, String sql) @Override public SelectBuilder getSelectBuilder(QuerySchema schema, String sql, boolean strictColumnList) + { + return getSelectBuilder(schema, sql, strictColumnList, null); + } + + @Override + public SelectBuilder getSelectBuilder(QuerySchema schema, String sql, boolean strictColumnList, @Nullable Map tableMap) { Query q = new Query(schema); q.setStrictColumnList(strictColumnList); - q.setTableMap(null); + q.setTableMap(tableMap); q.parse(sql); return getSelectBuilder(q); } @@ -2936,12 +2863,7 @@ public SQLFragment buildSqlFragment() } @Override - public SqlSelector buildSqlSelector() - { - return buildSqlSelector(Map.of()); - } - - private SqlSelector buildSqlSelector(@NotNull Map parameters) + public SqlSelector buildSqlSelector(@NotNull Map parameters) { SQLFragment sql = buildSqlFragment(); bindNamedParameters(sql, parameters); diff --git a/query/src/org/labkey/query/QueryTestCase.jsp b/query/src/org/labkey/query/QueryTestCase.jsp index e2027596840..a6a8ac4de8c 100644 --- a/query/src/org/labkey/query/QueryTestCase.jsp +++ b/query/src/org/labkey/query/QueryTestCase.jsp @@ -1306,7 +1306,7 @@ d,seven,twelve,day,month,date,duration,guid try { - Results rs = QueryService.get().select(schema, sql, null, true, true); + Results rs = QueryService.get().getSelectBuilder(schema, sql, true).select(true); assertNotNull(sql, rs); return rs; } @@ -1427,7 +1427,7 @@ d,seven,twelve,day,month,date,duration,guid } else { - try (Results rs = QueryService.get().getSelectBuilder(t).select()) + try (Results rs = QueryService.get().getSelectBuilder(t).select(true)) { assertNotNull(sql, rs); assertEquals(sql, Rsize, rs.getSize()); @@ -1464,7 +1464,7 @@ d,seven,twelve,day,month,date,duration,guid } else { - try (Results rs = QueryService.get().getSelectBuilder(t).select()) + try (Results rs = QueryService.get().getSelectBuilder(t).select(true)) { assertNotNull(sql, rs); assertEquals(sql, Rsize, rs.getSize()); @@ -1987,7 +1987,7 @@ d,seven,twelve,day,month,date,duration,guid UNION ALL SELECT 'g' as test, false as expected, array_contains_element( ARRAY['A','B'], 'X') as result UNION ALL - SELECT 'h' as test, true as expected, array_contains_any( ARRAY['\"A','X'], ARRAY['\"A','B'] ) as result + SELECT 'h' as test, true as expected, array_contains_any( ARRAY['"A','X'], ARRAY['"A','B'] ) as result UNION ALL SELECT 'i' as test, true as expected, array_is_same( ARRAY['A;','X'], ARRAY['A;','X'] ) as result """; @@ -1995,7 +1995,7 @@ d,seven,twelve,day,month,date,duration,guid Container container = JunitUtil.getTestContainer(); User user = TestContext.get().getUser(); var schema = DefaultSchema.get(user, container).getSchema("core"); - try (var rs =QueryService.get().select(schema, testSql)) + try (var rs = QueryService.get().getSelectBuilder(schema, testSql).select()) { while (rs.next()) { diff --git a/query/src/org/labkey/query/jdbc/QueryStatement.java b/query/src/org/labkey/query/jdbc/QueryStatement.java index bda2453af6a..36cb8f42e8c 100644 --- a/query/src/org/labkey/query/jdbc/QueryStatement.java +++ b/query/src/org/labkey/query/jdbc/QueryStatement.java @@ -55,7 +55,7 @@ public ResultSet executeQuery(String s) throws SQLException if (_log.isTraceEnabled()) cached = true; - _rs = QueryService.get().select(schema, s, null, true, cached); + _rs = QueryService.get().getSelectBuilder(schema, s, true).select(cached); if (_log.isTraceEnabled() && _rs instanceof CachedResultSet) { diff --git a/query/src/org/labkey/query/olap/BitSetQueryImpl.java b/query/src/org/labkey/query/olap/BitSetQueryImpl.java index 57842d316bc..4a72dbc0828 100644 --- a/query/src/org/labkey/query/olap/BitSetQueryImpl.java +++ b/query/src/org/labkey/query/olap/BitSetQueryImpl.java @@ -1786,7 +1786,7 @@ ResultSet execute(User user, String query) try { QuerySchema qs = DefaultSchema.get(user, container, Objects.toString(rolap.getQuerySchemaName(), "core")); - return QueryService.get().select(qs, query, null, true, false); + return QueryService.get().getSelectBuilder(qs, query, true).select(); } catch (QueryParseException|AssertionError x) { diff --git a/query/src/org/labkey/query/olap/metadata/RolapCachedCubeFactory.java b/query/src/org/labkey/query/olap/metadata/RolapCachedCubeFactory.java index d851056a000..6e8ebeb0b8c 100644 --- a/query/src/org/labkey/query/olap/metadata/RolapCachedCubeFactory.java +++ b/query/src/org/labkey/query/olap/metadata/RolapCachedCubeFactory.java @@ -165,7 +165,7 @@ void generateHierarchyMembers(CachedCube cube, HierarchyDef hdef, CachedCube._Hi CaseInsensitiveHashMap uniqueNameMap = new CaseInsensitiveHashMap<>(); String hierarchySql = rolap.getMembersSQL(hdef); - try (ResultSet rs = QueryService.get().select(schema, hierarchySql, null, true, false)) + try (ResultSet rs = QueryService.get().getSelectBuilder(schema, hierarchySql, true).select()) { // compute jdbcType for all key columns for (int l = 1; l < levelCount; l++) diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index ed4cd1c9ee7..b8e9963b4e1 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/study/src/org/labkey/study/controllers/StudyController.java @@ -7357,7 +7357,7 @@ public boolean handlePost(Object o, BindException errors) throws Exception cols.add(ti.getColumn("dateoffset")); SimpleFilter filter = new SimpleFilter(); filter.addCondition(ti.getColumn("container"), getContainer()); - ResultsFactory factory = ()->QueryService.get().select(ti, cols, filter, new Sort("participantid")); + ResultsFactory factory = ()->QueryService.get().getSelectBuilder(ti).columns(cols).filter(filter).sort(new Sort("participantid")).select(); // NOTE: TSVGridWriter closes PrintWriter and ResultSet try (TSVGridWriter writer = new TSVGridWriter(factory)) diff --git a/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java b/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java index c7cbb011e70..bdb7474f621 100644 --- a/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java +++ b/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java @@ -341,7 +341,7 @@ private Results getResults(ViewContext context, QueryView view, QuerySnapshotDef colMap.put(col.getFieldKey(), col); } } - return QueryService.get().select(tinfo, colMap.values(), filter, null); + return QueryService.get().getSelectBuilder(tinfo).columns(colMap.values()).filter(filter).select(); } @Nullable diff --git a/study/src/org/labkey/study/model/DatasetImportTestCase.jsp b/study/src/org/labkey/study/model/DatasetImportTestCase.jsp index b3cda9e371c..c2639902524 100644 --- a/study/src/org/labkey/study/model/DatasetImportTestCase.jsp +++ b/study/src/org/labkey/study/model/DatasetImportTestCase.jsp @@ -876,7 +876,7 @@ private void _testDatasetTransformExport(Study study) throws Throwable String alternateId = null; - try (ResultSet rs = QueryService.get().select(participantTableInfo, cols, SimpleFilter.createContainerFilter(study.getContainer()), null)) + try (ResultSet rs = QueryService.get().getSelectBuilder(participantTableInfo).columns(cols).filter(SimpleFilter.createContainerFilter(study.getContainer())).select()) { // store the ptid date offset and alternate ID for verification later int dateOffset = -1; diff --git a/visualization/src/org/labkey/visualization/VisualizationController.java b/visualization/src/org/labkey/visualization/VisualizationController.java index da47bdd687e..f1529480719 100644 --- a/visualization/src/org/labkey/visualization/VisualizationController.java +++ b/visualization/src/org/labkey/visualization/VisualizationController.java @@ -23,10 +23,10 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; import org.json.JSONObject; -import org.jetbrains.annotations.NotNull; import org.junit.Assert; import org.junit.Test; import org.labkey.api.action.Action; @@ -60,7 +60,6 @@ import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; import org.labkey.api.data.SqlSelector; -import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.data.views.DataViewProvider.EditInfo.ThumbnailType; import org.labkey.api.query.CustomView; @@ -1613,7 +1612,7 @@ public ApiResponse execute(SourceCountForm form, BindException errors) throws Ex } Map values = new HashMap<>(); - try (ResultSet rs = QueryService.get().select(userSchema, provider.getSourceCountSql(sources, members, colName))) + try (ResultSet rs = QueryService.get().getSelectBuilder(userSchema, provider.getSourceCountSql(sources, members, colName)).select()) { while (rs.next()) { @@ -1712,7 +1711,7 @@ public void testJacksonBinding() throws Exception assertEquals("table", m.getQueryName()); assertTrue(m.getRequireLeftJoin()); assertEquals("schema", m.getSchemaName()); - List values = m.getValues(); + List values = m.getValues(); assertNotNull(values); assertEquals(3, values.size()); assertEquals("1", values.get(0).toString()); diff --git a/visualization/src/org/labkey/visualization/sql/VisualizationCDSGenerator.java b/visualization/src/org/labkey/visualization/sql/VisualizationCDSGenerator.java index 6aca5ce2824..a310d66836a 100644 --- a/visualization/src/org/labkey/visualization/sql/VisualizationCDSGenerator.java +++ b/visualization/src/org/labkey/visualization/sql/VisualizationCDSGenerator.java @@ -537,7 +537,8 @@ Results getResults(VisDataRequest q) throws SQLGenerationException, SQLException BindException errors = new NullSafeBindException(q,"query"); String sql = gen.getSQL(errors); assertFalse(errors.hasErrors()); - return QueryService.get().getSelectBuilder(schema, sql, true).select(); + // Pass true for a cached, scrollable result set so callers can use getSize() and beforeFirst() + return QueryService.get().getSelectBuilder(schema, sql, true).select(true); } List> getColumnAliases(VisDataRequest q) throws SQLGenerationException, SQLException diff --git a/visualization/src/org/labkey/visualization/sql/VisualizationSQLGenerator.java b/visualization/src/org/labkey/visualization/sql/VisualizationSQLGenerator.java index 635dc289e43..f9565b5d43a 100644 --- a/visualization/src/org/labkey/visualization/sql/VisualizationSQLGenerator.java +++ b/visualization/src/org/labkey/visualization/sql/VisualizationSQLGenerator.java @@ -61,7 +61,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; /** * User: brittp @@ -108,11 +107,11 @@ public ViewContext getViewContext() } - private final Map _providers = new CaseInsensitiveHashMap<>(); + private final Map> _providers = new CaseInsensitiveHashMap<>(); - public VisualizationProvider ensureVisualizationProvider(String schemaName, VisualizationProvider.ChartType type) + public VisualizationProvider ensureVisualizationProvider(String schemaName, VisualizationProvider.ChartType type) { - VisualizationProvider provider = _providers.get(schemaName); + VisualizationProvider provider = _providers.get(schemaName); if (provider == null) { UserSchema userSchema = QueryService.get().getUserSchema(_viewContext.getUser(), _viewContext.getContainer(), schemaName); @@ -136,9 +135,9 @@ public VisualizationProvider ensureVisualizationProvider(String schemaName, Visu return provider; } - public @NotNull VisualizationProvider getVisualizationProvider(String schema) + public @NotNull VisualizationProvider getVisualizationProvider(String schema) { - VisualizationProvider provider = _providers.get(schema); + VisualizationProvider provider = _providers.get(schema); if (provider == null) throw new IllegalStateException("No provider configured for schema " + schema); return provider; @@ -213,7 +212,7 @@ else if ("visit".equalsIgnoreCase(timeAxis)) else throw new IllegalArgumentException("Only time charts are currently supported: expected 'time' property on each measure."); - VisualizationProvider provider = ensureVisualizationProvider(query.getSchemaName(), type); + VisualizationProvider provider = ensureVisualizationProvider(query.getSchemaName(), type); VisualizationIntervalColumn newInterval = null; switch (type) { @@ -256,6 +255,7 @@ else if ("visit".equalsIgnoreCase(timeAxis)) if (existingInterval.getStartCol() == newInterval.getStartCol() && existingInterval.getInterval() == newInterval.getInterval()) { foundMatch = true; + break; } } if (!foundMatch) @@ -344,7 +344,7 @@ else if ("visit".equalsIgnoreCase(timeAxis)) { _groupBys.addAll(groupBys.stream() .map(additionalSelectInfo -> _columnFactory.create(getViewContext(), additionalSelectInfo)) - .collect(Collectors.toList())); + .toList()); } ensureJoinColumns(); @@ -497,7 +497,7 @@ private String wrapInGroupBy(IVisualizationSourceQuery joinQuery, List provider : _providers.values()) { provider.appendAggregates(groupByAndOrderBySQL, columnAliases, _intervals, "x", joinQuery, false); provider.appendAggregates(selectSQL, columnAliases, _intervals, "x", joinQuery, true); @@ -613,7 +613,7 @@ private String getSubselectSQL(IVisualizationSourceQuery parentQuery, Visualizat boolean isJoinColumn = true; for (VisualizationSourceColumn select : entry.getValue()) { - VisualizationProvider provider = getVisualizationProvider(select.getSchemaName()); + VisualizationProvider provider = getVisualizationProvider(select.getSchemaName()); if (!provider.isJoinColumn(select, getViewContext().getContainer())) { isJoinColumn = false; @@ -926,7 +926,7 @@ public List getColumns() // The default column mapping references the first available valid alias: for (Map.Entry> entry : allAliases.entrySet()) { - result.addAll(entry.getValue().stream().collect(Collectors.toList())); + result.addAll(entry.getValue().stream().toList()); } return new ArrayList<>(result); @@ -949,7 +949,7 @@ public List> getColumnAliases() { for (VisualizationSourceColumn select : columnSet) { - VisualizationProvider provider = getVisualizationProvider(select.getSchemaName()); + VisualizationProvider provider = getVisualizationProvider(select.getSchemaName()); if (!provider.isJoinColumn(select, getViewContext().getContainer())) result.add(select.toJSON()); } @@ -979,7 +979,7 @@ public Map getColumnMapping() { for (VisualizationSourceColumn select : entry.getValue()) { - VisualizationProvider provider = getVisualizationProvider(select.getSchemaName()); + VisualizationProvider provider = getVisualizationProvider(select.getSchemaName()); if (!provider.isJoinColumn(select, getViewContext().getContainer())) colMap.put(select.getAlias(), select.getAlias()); } @@ -1044,7 +1044,7 @@ public String getFilterDescription() return builder.toString(); } - static Object pickFirst(Collection set) + static T pickFirst(Collection set) { set.iterator().hasNext(); return set.iterator().next(); @@ -1126,7 +1126,8 @@ Results getResults(VisDataRequest q) throws SQLGenerationException, SQLException VisualizationSQLGenerator gen = getVSQL(q); UserSchema schema = new VisTestSchema(context.getUser(), context.getContainer()); String sql = gen.getSQL(); - return QueryService.get().getSelectBuilder(schema, sql, true).select(); + // Pass true for a cached, scrollable result set so callers can use getSize() and beforeFirst() + return QueryService.get().getSelectBuilder(schema, sql, true).select(true); } @@ -1204,7 +1205,7 @@ public void testOneTableWithValues() throws Exception // it seems strange to filter by using a sort? but OK VisDataRequest.Measure ptidList = new VisDataRequest.Measure("vis_junit", "demographics", "participantid"); - ptidList.setValues((List)VisTestSchema.humans); + ptidList.setValues(VisTestSchema.humans); q.addSort(ptidList); try (ResultsImpl r = (ResultsImpl)getResults(q)) {