From 2e6b2361cdb8c147b50a8937ca83a45677475efb Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 24 May 2026 09:25:09 -0700 Subject: [PATCH 1/2] GitHub Issue 1188: Stop JDBC caching by default in QueryService --- .../api/assay/AbstractAssayProvider.java | 2 +- api/src/org/labkey/api/data/DataRegion.java | 2 +- .../org/labkey/api/data/GroupTableInfo.java | 2 +- .../labkey/api/data/NestedRenderContext.java | 6 +- api/src/org/labkey/api/data/Table.java | 2 +- .../org/labkey/api/data/TableSelector.java | 8 +- .../QueryDataIteratorBuilder.java | 2 +- .../org/labkey/api/query/QueryService.java | 25 +++-- .../api/study/assay/SpecimenForeignKey.java | 2 +- .../api/assay/nab/query/NAbSpecimenTable.java | 2 +- .../org/labkey/assay/plate/PlateManager.java | 4 +- .../labkey/assay/plate/PlateManagerTest.java | 2 +- .../assay/plate/data/WellTriggerFactory.java | 4 +- .../labkey/core/query/CoreQuerySchema.java | 5 +- .../labkey/experiment/api/LineageTest.java | 2 +- .../experiment/lineage/LineagePerfTest.java | 2 +- .../org/labkey/issue/model/IssueManager.java | 2 +- .../src/org/labkey/list/model/ListWriter.java | 4 +- .../org/labkey/query/QueryServiceImpl.java | 95 ++++++------------- .../labkey/query/QueryServiceImplTestCase.jsp | 28 ++---- .../query/controllers/SqlController.java | 2 +- .../org/labkey/query/sql/QuerySelectView.java | 3 +- .../specimen/SpecimenRequestManager.java | 2 +- .../study/assay/StudyPublishManager.java | 2 +- .../study/writer/DatasetDataWriter.java | 2 +- .../writer/DefaultStudyDesignWriter.java | 2 +- .../VisualizationController.java | 2 +- .../sql/VisualizationCDSGenerator.java | 2 +- .../sql/VisualizationSQLGenerator.java | 2 +- 29 files changed, 84 insertions(+), 136 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayProvider.java b/api/src/org/labkey/api/assay/AbstractAssayProvider.java index 724af962d08..b8c3c0ca421 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayProvider.java +++ b/api/src/org/labkey/api/assay/AbstractAssayProvider.java @@ -240,7 +240,7 @@ public ActionURL linkToStudy(User user, Container assayDataContainer, ExpProtoco ColumnInfo rowIdColumn = columns.get(objectIdFK); ColumnInfo runLSIDColumn = columns.get(runLSIDFK); - SQLFragment sql = QueryService.get().getSelectSQL(dataTable, columns.values(), filter, null, Table.ALL_ROWS, Table.NO_OFFSET, false); + SQLFragment sql = QueryService.get().getSelectBuilder(dataTable).columns(columns.values()).filter(filter).buildSqlFragment(); List> dataMaps = new ArrayList<>(); Container sourceContainer = null; diff --git a/api/src/org/labkey/api/data/DataRegion.java b/api/src/org/labkey/api/data/DataRegion.java index 5a6ea88828a..140ba6844bc 100644 --- a/api/src/org/labkey/api/data/DataRegion.java +++ b/api/src/org/labkey/api/data/DataRegion.java @@ -2128,7 +2128,7 @@ private void renderMultipleUpdateForm(RenderContext ctx, HtmlWriter out) for (Map.Entry entry : selectKeyMap.entrySet()) { ColumnInfo col = entry.getValue(); - SQLFragment selectSql = service.getSelectSQL(table, Collections.singletonList(col), pkFilter, null, Table.ALL_ROWS, Table.NO_OFFSET, false, queryLogging); + SQLFragment selectSql = service.getSelectBuilder(table).columns(Collections.singletonList(col)).filter(pkFilter).queryLogging(queryLogging).buildSqlFragment(); SQLFragment sql = new SQLFragment("SELECT DISTINCT ").appendIdentifier(col.getAlias()).append(" AS value FROM ("); sql.append(selectSql); diff --git a/api/src/org/labkey/api/data/GroupTableInfo.java b/api/src/org/labkey/api/data/GroupTableInfo.java index 77413d25953..039eb147b06 100644 --- a/api/src/org/labkey/api/data/GroupTableInfo.java +++ b/api/src/org/labkey/api/data/GroupTableInfo.java @@ -95,7 +95,7 @@ public SQLFragment getFromSQL() sql.append("\nFROM (\n"); TableInfo source = getSourceTable(); - sql.append(QueryService.get().getSelectSQL(source, getDistinctColumns(), _sourceFilter, null, Table.ALL_ROWS, Table.NO_OFFSET, false)); + sql.append(QueryService.get().getSelectBuilder(source).columns(getDistinctColumns()).filter(_sourceFilter).buildSqlFragment()); sql.append("\n) AS "); sql.append(ALIAS_SOURCE); diff --git a/api/src/org/labkey/api/data/NestedRenderContext.java b/api/src/org/labkey/api/data/NestedRenderContext.java index 7b0820a3c71..d8ed55a4838 100644 --- a/api/src/org/labkey/api/data/NestedRenderContext.java +++ b/api/src/org/labkey/api/data/NestedRenderContext.java @@ -111,8 +111,8 @@ public SimpleFilter buildFilter(TableInfo tinfo, List displayColumns fromSQL.append(" ) FilterOnly "); Collection cols = Collections.singletonList(groupColumn); - SQLFragment withoutSort = QueryService.get().getSelectSQL(tinfo, cols, new SimpleFilter(), new Sort(), Table.ALL_ROWS, Table.NO_OFFSET, false); - SQLFragment withSort = QueryService.get().getSelectSQL(tinfo, cols, new SimpleFilter(), groupingSort, Table.ALL_ROWS, Table.NO_OFFSET, false); + SQLFragment withoutSort = QueryService.get().getSelectBuilder(tinfo).columns(cols).buildSqlFragment(); + SQLFragment withSort = QueryService.get().getSelectBuilder(tinfo).columns(cols).sort(groupingSort).buildSqlFragment(); // Figure out what the ORDER BY is String sortSQL = withSort.getSQL().substring(withSort.getSQL().toUpperCase().lastIndexOf("ORDER BY")); @@ -205,7 +205,7 @@ private ColumnInfo appendFromSQL(TableInfo tinfo, String dataRegionName, Sort so // We need to stick the GROUP BY before the ORDER BY. QueryService won't help us generate // the GROUP BY, so get the query with and without the ORDER BY - SQLFragment withoutSort = QueryService.get().getSelectSQL(tinfo, cols, filter, new Sort(), Table.ALL_ROWS, Table.NO_OFFSET, false); + SQLFragment withoutSort = QueryService.get().getSelectBuilder(tinfo).columns(cols).filter(filter).buildSqlFragment(); sql.append(withoutSort); diff --git a/api/src/org/labkey/api/data/Table.java b/api/src/org/labkey/api/data/Table.java index 8ecb3cd9655..9966722ec06 100644 --- a/api/src/org/labkey/api/data/Table.java +++ b/api/src/org/labkey/api/data/Table.java @@ -1240,7 +1240,7 @@ public static void truncate(TableInfo table) public static SQLFragment getSelectSQL(TableInfo table, @Nullable Collection columns, @Nullable Filter filter, @Nullable Sort sort) { - return QueryService.get().getSelectSQL(table, columns, filter, sort, ALL_ROWS, NO_OFFSET, false); + return QueryService.get().getSelectBuilder(table).columns(columns).filter(filter).sort(sort).buildSqlFragment(); } public static void ensureRequiredColumns(TableInfo table, Map cols, @Nullable Filter filter, @Nullable Sort sort, @Nullable List aggregates) diff --git a/api/src/org/labkey/api/data/TableSelector.java b/api/src/org/labkey/api/data/TableSelector.java index 556344f0a27..343c78ec97e 100644 --- a/api/src/org/labkey/api/data/TableSelector.java +++ b/api/src/org/labkey/api/data/TableSelector.java @@ -618,9 +618,9 @@ public SQLFragment getSql() { Map map = getDisplayColumnsList(_columns); - // QueryService.getSelectSQL() also calls ensureRequiredColumns, so this call is redundant. However, we - // need to know the actual select columns (e.g., if the caller is building a Results) and getSelectSQL() - // doesn't return them. TODO: Provide a way to return the selected columns from getSelectSQL() + // SelectBuilderImpl.buildSqlFragment() also calls ensureRequiredColumns, so this call is redundant. + // However, we need to know the actual select columns (e.g., if the caller is building a Results) + // and buildSqlFragment() doesn't return them. Table.ensureRequiredColumns(_table, map, _filter, _sort, null); _columns = map.values(); } @@ -646,7 +646,7 @@ public SQLFragment getSql() } int selectMaxRows = (Table.ALL_ROWS == _maxRows || Table.NO_ROWS == _maxRows) ? _maxRows : (int)_scrollOffset + _maxRows + _extraRows; - SQLFragment sql = QueryService.get().getSelectSQL(_table, _columns, _filter, _sort, selectMaxRows, selectOffset, forceSort, getQueryLogging()); + SQLFragment sql = QueryService.get().getSelectBuilder(_table).columns(_columns).filter(_filter).sort(_sort).maxRows(selectMaxRows).offset(selectOffset).forceSort(forceSort).queryLogging(getQueryLogging()).buildSqlFragment(); // This is for SAS, which doesn't support a SQL LIMIT syntax, so we must set Statement.maxRows() instead _statementMaxRows = _table.getSqlDialect().requiresStatementMaxRows() ? selectMaxRows : null; diff --git a/api/src/org/labkey/api/dataiterator/QueryDataIteratorBuilder.java b/api/src/org/labkey/api/dataiterator/QueryDataIteratorBuilder.java index c2c938ef4d8..4015daa6058 100644 --- a/api/src/org/labkey/api/dataiterator/QueryDataIteratorBuilder.java +++ b/api/src/org/labkey/api/dataiterator/QueryDataIteratorBuilder.java @@ -162,7 +162,7 @@ public DataIterator getDataIterator(DataIteratorContext context) try { var select = qs.getSelectBuilder(t).columns(selectCols).filter(_filter); - ResultSet rs = select.select(_parameters, false); + ResultSet rs = select.select(false, _parameters); return new ResultSetDataIterator(rs, context); } catch (QueryParseException x) diff --git a/api/src/org/labkey/api/query/QueryService.java b/api/src/org/labkey/api/query/QueryService.java index 63b143149a8..f252c17168e 100644 --- a/api/src/org/labkey/api/query/QueryService.java +++ b/api/src/org/labkey/api/query/QueryService.java @@ -259,25 +259,16 @@ default ResultSet select(QuerySchema schema, String sql) /* 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} */ - Results selectResults(@NotNull QuerySchema schema, String sql, @Nullable Map tableMap, Map parameters, 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(); } - /** superseded by {@link QueryService#getSelectBuilder} */ - Results select(TableInfo table, Collection columns, @Nullable Filter filter, @Nullable Sort sort, Map parameters, boolean cached); - - /** superseded by {@link QueryService#getSelectBuilder} */ - SQLFragment getSelectSQL(TableInfo table, @Nullable Collection columns, @Nullable Filter filter, @Nullable Sort sort, int maxRows, long offset, boolean forceSort); - /** superseded by {@link QueryService#getSelectBuilder} */ - SQLFragment getSelectSQL(TableInfo table, @Nullable Collection columns, @Nullable Filter filter, @Nullable Sort sort, int maxRows, long offset, boolean forceSort, @NotNull QueryLogging queryLogging); - 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); MutableColumnInfo createQueryExpressionColumn(TableInfo table, FieldKey key, String labKeySql, ColumnType columnType); MutableColumnInfo createQueryExpressionColumn(TableInfo table, FieldKey key, FieldKey wrapped, ColumnType columnType); @@ -697,13 +688,19 @@ default SelectBuilder columns(ColumnInfo... cols) SelectBuilder forceSort(boolean b); SelectBuilder queryLogging(QueryLogging queryLogging); SelectBuilder distinct(boolean b); + /** Enable PostgreSQL JDBC-level result buffering. Rarely needed; defaults to false (streaming). */ + SelectBuilder jdbcCaching(boolean jdbcCaching); SQLFragment buildSqlFragment(); - SqlSelector buildSqlSelector(@Nullable Map parameters); - Results select(@Nullable Map parameters, boolean cache); + SqlSelector buildSqlSelector(); + Results select(boolean labkeyCachedResultSet, @NotNull Map parameters); + default Results select(boolean labkeyCachedResultSet) + { + return select(labkeyCachedResultSet, Map.of()); + } default Results select() { - return select(Map.of(), true); + return select(true); } QueryLogging getQueryLogging(); diff --git a/api/src/org/labkey/api/study/assay/SpecimenForeignKey.java b/api/src/org/labkey/api/study/assay/SpecimenForeignKey.java index 64ee0823876..b940fcd1227 100644 --- a/api/src/org/labkey/api/study/assay/SpecimenForeignKey.java +++ b/api/src/org/labkey/api/study/assay/SpecimenForeignKey.java @@ -363,7 +363,7 @@ public SQLFragment _declareJoinsAssayAndVial(String parentAlias, ColumnInfo fore // Select all the assay-side specimen columns that we'll need to do the comparison // TODO ContainerFilter make sure all callers of new SpecimenForeignKey() pass in appropriately constructed _assayDataTable // ((ContainerFilterable)_assayDataTable).setContainerFilter(foreignKey.getParentTable().getContainerFilter()); - SQLFragment targetStudySQL = QueryService.get().getSelectSQL(_assayDataTable, _assayColumns.values(), null, null, Table.ALL_ROWS, Table.NO_OFFSET, false); + SQLFragment targetStudySQL = QueryService.get().getSelectBuilder(_assayDataTable).columns(_assayColumns.values()).buildSqlFragment(); sql.append(targetStudySQL); String baseAlias = getBaseAlias(parentAlias, foreignKey.getAlias()); diff --git a/assay/api-src/org/labkey/api/assay/nab/query/NAbSpecimenTable.java b/assay/api-src/org/labkey/api/assay/nab/query/NAbSpecimenTable.java index 2b6267c8a0f..d0be713818c 100644 --- a/assay/api-src/org/labkey/api/assay/nab/query/NAbSpecimenTable.java +++ b/assay/api-src/org/labkey/api/assay/nab/query/NAbSpecimenTable.java @@ -143,7 +143,7 @@ private SQLFragment getPercentNeutralizationInitialDilution() if (samplesTable != null && samplesTable.getColumn("InitialDilution") != null) { List columns = Arrays.asList(samplesTable.getColumn("LSID"), samplesTable.getColumn("InitialDilution")); - SQLFragment samplesSql = QueryService.get().getSelectSQL(samplesTable, columns, null, null, Table.ALL_ROWS, 0, false); + SQLFragment samplesSql = QueryService.get().getSelectBuilder(samplesTable).columns(columns).buildSqlFragment(); return sql.append(" LEFT JOIN (").append(samplesSql).append(" ) x ON x.LSID = ").append(ExprColumn.STR_TABLE_ALIAS + ".SpecimenLsid") .append(" WHERE dd.RunDataId = ").append(ExprColumn.STR_TABLE_ALIAS + ".RowId") .append(" AND dd.Dilution = x.InitialDilution)"); diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index 4c1da61c95a..b6846384f7c 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -4248,7 +4248,7 @@ private long getReplicateGroupCount(@NotNull UserSchema plateSchema, @NotNull Lo FROM plate.Well WHERE PlateId.PlateSet.RowId = %s AND ReplicateGroup IS NOT NULL """, plateSetRowId); - return QueryService.get().getSelectBuilder(plateSchema, labkeySql).buildSqlSelector(null).getRowCount(); + return QueryService.get().getSelectBuilder(plateSchema, labkeySql).buildSqlSelector().getRowCount(); } private String getReplicateGroupLabKeySql(@NotNull UserSchema plateSchema, @NotNull Long plateSetRowId) @@ -4349,7 +4349,7 @@ private String getSampleGroupLabKeySql(@NotNull Long plateSetRowId, boolean incl private long getSampleGroupCount(@NotNull UserSchema plateSchema, @NotNull Long plateSetRowId) { String labkeySql = getSampleGroupLabKeySql(plateSetRowId, false); - return QueryService.get().getSelectBuilder(plateSchema, labkeySql).buildSqlSelector(null).getRowCount(); + return QueryService.get().getSelectBuilder(plateSchema, labkeySql).buildSqlSelector().getRowCount(); } private void validatePlateSetSampleGroups(Container container, User user, @NotNull Long plateSetRowId) throws ValidationException diff --git a/assay/src/org/labkey/assay/plate/PlateManagerTest.java b/assay/src/org/labkey/assay/plate/PlateManagerTest.java index 77efd75ea70..273f3608163 100644 --- a/assay/src/org/labkey/assay/plate/PlateManagerTest.java +++ b/assay/src/org/labkey/assay/plate/PlateManagerTest.java @@ -2096,7 +2096,7 @@ private Map getWellRow(long plateRowId, @NotNull String position return QueryService.get().getSelectBuilder(wellTable) .columns(getWellTableColumns(wellTable).values()) .filter(filter) - .buildSqlSelector(null) + .buildSqlSelector() .getMap(); } diff --git a/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java b/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java index bc9f4a0f7b0..c4522c6d7c5 100644 --- a/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java +++ b/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java @@ -413,7 +413,7 @@ private Map getWellTypes(Container container, User user, long plat UserSchema schema = QueryService.get().getUserSchema(user, container, "plate"); SQLFragment sql = new SQLFragment("SELECT RowId, Type FROM plate.Well WHERE PlateId = ?").add(plateRowId); QueryService.get().getSelectBuilder(schema, sql.toDebugString()) - .buildSqlSelector(null) + .buildSqlSelector() .forEach(r -> map.put(r.getLong(WellTable.Column.RowId.name()), r.getString(WellTable.Column.Type.name()))); return map; @@ -425,7 +425,7 @@ private Map getWellReplicateGroups(Container container, User user, UserSchema schema = QueryService.get().getUserSchema(user, container, "plate"); SQLFragment sql = new SQLFragment("SELECT RowId, ReplicateGroup FROM plate.Well WHERE PlateId = ?").add(plateRowId); QueryService.get().getSelectBuilder(schema, sql.toDebugString()) - .buildSqlSelector(null) + .buildSqlSelector() .forEach(r -> map.put(r.getLong(WellTable.Column.RowId.name()), r.getString(WellTable.Column.ReplicateGroup.name()))); return map; diff --git a/core/src/org/labkey/core/query/CoreQuerySchema.java b/core/src/org/labkey/core/query/CoreQuerySchema.java index 70b22fdca49..2d21c7ec641 100644 --- a/core/src/org/labkey/core/query/CoreQuerySchema.java +++ b/core/src/org/labkey/core/query/CoreQuerySchema.java @@ -779,11 +779,10 @@ public static boolean requiresProfileUpdate(User user) settings.setBaseFilter(new SimpleFilter(FieldKey.fromParts("UserId"), user.getUserId())); - Map params = Collections.emptyMap(); TableInfo table = schema.getTable(CoreQuerySchema.USERS_TABLE_NAME); - try (Results results = QueryService.get().select(table, table.getColumns(), - new SimpleFilter(FieldKey.fromParts("UserId"), user.getUserId()), null, params, true)) + try (Results results = QueryService.get().getSelectBuilder(table).filter( + new SimpleFilter(FieldKey.fromParts("UserId"), user.getUserId())).select(true)) { if (results.next()) { diff --git a/experiment/src/org/labkey/experiment/api/LineageTest.java b/experiment/src/org/labkey/experiment/api/LineageTest.java index e33eaf5b8c8..f0074bffbfd 100644 --- a/experiment/src/org/labkey/experiment/api/LineageTest.java +++ b/experiment/src/org/labkey/experiment/api/LineageTest.java @@ -243,7 +243,7 @@ public void testDeriveDuringImport() throws Exception "FROM exp.data." + firstDataClassName + " AS dc\n" + "ORDER BY dc.RowId\n"; - try (Results rs = QueryService.get().selectResults(schema, sql, null, null, true, true)) + try (Results rs = QueryService.get().getSelectBuilder(schema, sql, true).select()) { RenderContext ctx = new RenderContext(new ViewContext()); ctx.getViewContext().setRequest(TestContext.get().getRequest()); diff --git a/experiment/src/org/labkey/experiment/lineage/LineagePerfTest.java b/experiment/src/org/labkey/experiment/lineage/LineagePerfTest.java index f7781b9f39b..d7c6d8abb67 100644 --- a/experiment/src/org/labkey/experiment/lineage/LineagePerfTest.java +++ b/experiment/src/org/labkey/experiment/lineage/LineagePerfTest.java @@ -422,7 +422,7 @@ private void lineageQueries(String prefix, CPUTimer lineageQuery, CPUTimer linea "FROM samples.MySamples AS ss\n"; final UserSchema schema = QueryService.get().getUserSchema(_user, _container, "samples"); - final TableSelector ts = QueryService.get().selector(schema, sql); + final SqlSelector ts = QueryService.get().getSelectBuilder(schema, sql).buildSqlSelector(); final ExpLineageOptions opt = new ExpLineageOptions(); final ViewBackgroundInfo info = new ViewBackgroundInfo(_container, _user, null); diff --git a/issues/src/org/labkey/issue/model/IssueManager.java b/issues/src/org/labkey/issue/model/IssueManager.java index 4254960b59d..9d6958c2bd7 100644 --- a/issues/src/org/labkey/issue/model/IssueManager.java +++ b/issues/src/org/labkey/issue/model/IssueManager.java @@ -257,7 +257,7 @@ private static IssueObject _getIssue(@Nullable Container c, User user, int issue if (table != null) { var select = QueryService.get().getSelectBuilder(table).filter(filter); - try (Results rs = select.select(Map.of(), false)) + try (Results rs = select.select(false)) { Map rowMap = new CaseInsensitiveHashMap<>(); if (rs.next()) diff --git a/list/src/org/labkey/list/model/ListWriter.java b/list/src/org/labkey/list/model/ListWriter.java index 29515196266..672d0db392e 100644 --- a/list/src/org/labkey/list/model/ListWriter.java +++ b/list/src/org/labkey/list/model/ListWriter.java @@ -181,7 +181,7 @@ private void writeLists(List> listsToExport, Virtua // NOTE: TSVGridWriter generates and closes Results - try (TSVGridWriter tsvWriter = new TSVGridWriter(()->QueryService.get().getSelectBuilder(ti).columns(columns).sort(sort).select(null, false), displayColumns)) + try (TSVGridWriter tsvWriter = new TSVGridWriter(()->QueryService.get().getSelectBuilder(ti).columns(columns).sort(sort).select(false), displayColumns)) { tsvWriter.setApplyFormats(false); tsvWriter.setColumnHeaderType(ColumnHeaderType.ImportField); // Issue 53431 @@ -292,7 +292,7 @@ private void writeAttachments(TableInfo ti, ListDefinition def, Container c, Vir List selectColumns = new ArrayList<>(attachmentColumns); selectColumns.addFirst(ti.getColumn("EntityId")); - try (ResultSet rs = QueryService.get().getSelectBuilder(ti).columns(selectColumns).select(null, false)) + try (ResultSet rs = QueryService.get().getSelectBuilder(ti).columns(selectColumns).select(false)) { while (rs.next()) { diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 2b0a8abb4c8..f0d60c26c6a 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -374,7 +374,7 @@ public WhereClause createFilterClause(@NotNull FieldKey fieldKey, Object value) } }; - private static SQLFragment getColumnInSql(@NotNull FieldKey fieldKey, Object value, User user, Container container, Map columnMap, @NotNull List selectColumns, boolean negate) + private static SQLFragment getColumnInSql(@NotNull FieldKey fieldKey, Object value, User user, Container container, Map columnMap, boolean negate) { if (user == null || container == null) throw new NotFoundException("Invalid context"); @@ -423,7 +423,7 @@ public SimpleFilter.FilterClause createFilterClause(@NotNull FieldKey fieldKey, @Override public SQLFragment toSQLFragment(Map columnMap, SqlDialect dialect) { - return getColumnInSql(fieldKey, value, user, container, columnMap, _selectColumns, false); + return getColumnInSql(fieldKey, value, user, container, columnMap, false); } }; } @@ -445,7 +445,7 @@ public SimpleFilter.FilterClause createFilterClause(@NotNull FieldKey fieldKey, @Override public SQLFragment toSQLFragment(Map columnMap, SqlDialect dialect) { - return getColumnInSql(fieldKey, value, user, container, columnMap, _selectColumns, true); + return getColumnInSql(fieldKey, value, user, container, columnMap, true); } }; } @@ -2733,19 +2733,7 @@ public Results select(@NotNull QuerySchema schema, String sql, @Nullable Map tableMap, Map parameters, boolean strictColumnList, boolean cached) - { - Query q = new Query(schema); - q.setStrictColumnList(strictColumnList); - q.setTableMap(tableMap); - q.parse(sql); - - return getSelectBuilder(q).select(parameters, cached); + return getSelectBuilder(q).select(cached); } @@ -2809,45 +2797,6 @@ public void validateNamedParameters(SQLFragment frag) } } - @Override - public Results select(TableInfo table, Collection columns, @Nullable Filter filter, @Nullable Sort sort, Map parameters, boolean cache) - { - return getSelectBuilder(table) - .columns(columns) - .filter(filter) - .sort(sort) - .select(parameters, cache); - } - - @Override - public SQLFragment getSelectSQL(TableInfo table, @Nullable Collection selectColumns, @Nullable Filter filter, @Nullable Sort sort, - int maxRows, long offset, boolean forceSort) - { - return getSelectBuilder(table) - .columns(selectColumns) - .filter(filter) - .sort(sort) - .maxRows(maxRows) - .offset(offset) - .forceSort(forceSort) - .buildSqlFragment(); - } - - @Override - public SQLFragment getSelectSQL(TableInfo table, @Nullable Collection selectColumns, @Nullable Filter filter, @Nullable Sort sort, - int maxRows, long offset, boolean forceSort, @NotNull QueryLogging queryLogging) - { - return getSelectBuilder(table) - .columns(selectColumns) - .filter(filter) - .sort(sort) - .maxRows(maxRows) - .offset(offset) - .forceSort(forceSort) - .queryLogging(queryLogging) - .buildSqlFragment(); - } - @Override public SelectBuilder getSelectBuilder(TableInfo table) { @@ -2861,9 +2810,15 @@ public SelectBuilder getSelectBuilder(Query query) @Override public SelectBuilder getSelectBuilder(QuerySchema schema, String sql) + { + return getSelectBuilder(schema, sql, false); + } + + @Override + public SelectBuilder getSelectBuilder(QuerySchema schema, String sql, boolean strictColumnList) { Query q = new Query(schema); - q.setStrictColumnList(false); + q.setStrictColumnList(strictColumnList); q.setTableMap(null); q.parse(sql); return getSelectBuilder(q); @@ -2882,6 +2837,7 @@ class SelectBuilderImpl implements SelectBuilder boolean forceSort = false; QueryLogging queryLogging = new QueryLogging(); boolean distinct = false; + boolean jdbcCaching = false; SelectBuilderImpl(TableInfo table) { @@ -2958,6 +2914,13 @@ public SelectBuilder distinct(boolean b) return this; } + @Override + public SelectBuilder jdbcCaching(boolean jdbcCaching) + { + this.jdbcCaching = jdbcCaching; + return this; + } + @Override public SQLFragment buildSqlFragment() { @@ -2973,7 +2936,12 @@ public SQLFragment buildSqlFragment() } @Override - public SqlSelector buildSqlSelector(@Nullable Map parameters) + public SqlSelector buildSqlSelector() + { + return buildSqlSelector(Map.of()); + } + + private SqlSelector buildSqlSelector(@NotNull Map parameters) { SQLFragment sql = buildSqlFragment(); bindNamedParameters(sql, parameters); @@ -2983,10 +2951,10 @@ public SqlSelector buildSqlSelector(@Nullable Map parameters) } @Override - public Results select(@Nullable Map parameters, boolean cache) + public Results select(boolean labkeyCachedResultSet, @NotNull Map parameters) { - SqlSelector selector = buildSqlSelector(parameters).setJdbcCaching(cache); - ResultSet rs = selector.getResultSet(cache, cache); + SqlSelector selector = buildSqlSelector(parameters).setJdbcCaching(jdbcCaching); + ResultSet rs = selector.getResultSet(labkeyCachedResultSet, labkeyCachedResultSet); // Keep track of whether we've successfully created the ResultSetImpl to return. If not, we should // close the underlying ResultSet before returning since it won't be accessible anywhere else @@ -3034,8 +3002,7 @@ public MutableColumnInfo createQueryExpressionColumn(TableInfo table, FieldKey k public MutableColumnInfo createQueryExpressionColumn(TableInfo table, FieldKey key, FieldKey wrapped, ColumnType columnType) { // TODO short-circuit parsing/binding in this code path - var ret = new CalculatedExpressionColumn(table, key, LabKeySql.quoteIdentifier(wrapped.getName()), columnType); - return ret; + return new CalculatedExpressionColumn(table, key, LabKeySql.quoteIdentifier(wrapped.getName()), columnType); } /** Compute and set the metadata for this column based on the source expression and the xml override */ @@ -3556,7 +3523,7 @@ private TableInfo _analyzeTableInfo(DependencyObject from, TableInfo source, Set TableInfo target = fk.getLookupTableInfo(); if (null == target) continue; - if (!_includeLookupDependency(source, target)) + if (!_includeLookupDependency(target)) continue; var type = target instanceof QueryTableInfo ? DependencyType.Query : DependencyType.Table; Container c = fk.getLookupContainer(); @@ -3580,7 +3547,7 @@ private TableInfo _analyzeTableInfo(DependencyObject from, TableInfo source, Set final static SchemaKey coreSchemaKey = SchemaKey.fromParts("core"); - private boolean _includeLookupDependency(TableInfo from, TableInfo to) + private boolean _includeLookupDependency(TableInfo to) { // ignore core schema if (to.getUserSchema() == null) diff --git a/query/src/org/labkey/query/QueryServiceImplTestCase.jsp b/query/src/org/labkey/query/QueryServiceImplTestCase.jsp index 6c46104b2bd..c9cd240d52e 100644 --- a/query/src/org/labkey/query/QueryServiceImplTestCase.jsp +++ b/query/src/org/labkey/query/QueryServiceImplTestCase.jsp @@ -64,37 +64,27 @@ var xyz = wrapped.getColumn("xyz"); // test choose PK - SQLFragment test1 = QueryServiceImpl.get().getSelectSQL(wrapped, - List.of(C,B,A), null, - null, 1000, 0, true); + SQLFragment test1 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(C,B,A)).buildSqlFragment(); assertTrue(test1.toDebugString().contains("ORDER BY A ASC")); assertTrue(isSorted(test1,3)); // test choose non-PK - SQLFragment test2 = QueryServiceImpl.get().getSelectSQL(wrapped, - List.of(C), null, - null, 1000, 0, true); + SQLFragment test2 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(C)).buildSqlFragment(); assertTrue(test2.toDebugString().contains("ORDER BY C ASC")); assertTrue(isSorted(test2,1)); // test explicit in select list - SQLFragment test3 = QueryServiceImpl.get().getSelectSQL(wrapped, - List.of(A,C), null, - new Sort("C"), 1000, 0, true); + SQLFragment test3 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,C)).sort(new Sort("C")).buildSqlFragment(); assertTrue(test3.toDebugString().contains("ORDER BY C ASC")); assertTrue(isSorted(test3,2)); // test explicit not in select list - SQLFragment test4 = QueryServiceImpl.get().getSelectSQL(wrapped, - List.of(A,B,xyz), null, - new Sort("C"), 1000, 0, true); + SQLFragment test4 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,xyz)).sort(new Sort("C")).buildSqlFragment(); assertTrue(test4.toDebugString().contains("ORDER BY C ASC")); assertTrue(isSorted(test4,3)); // test sortFieldKeys - SQLFragment test5 = QueryServiceImpl.get().getSelectSQL(wrapped, - List.of(A,B,xyz), null, - new Sort("B"), 1000, 0, true); + SQLFragment test5 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,xyz)).sort(new Sort("B")).buildSqlFragment(); assertFalse(test5.toDebugString().contains("ORDER BY B ASC")); assertTrue(test5.toDebugString().contains("ORDER BY C ASC")); assertTrue(isSorted(test5,3)); @@ -103,18 +93,14 @@ (wrapped.getMutableColumn("B")).setSortFieldKeys(Arrays.asList(new FieldKey(null,"D"))); // implicit sort, sort by B because D not found - SQLFragment test6 = QueryServiceImpl.get().getSelectSQL(wrapped, - List.of(B), null, - null, 1000, 0, true); + SQLFragment test6 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(B)).buildSqlFragment(); assertFalse(test6.toDebugString().contains("ORDER BY D ASC")); assertTrue(test6.toDebugString().contains("ORDER BY B ASC")); assertTrue(isSorted(test6,1)); // explicit sort, sort by B because D not found (wrapped.getMutableColumn("B")).setSortFieldKeys(Arrays.asList(new FieldKey(null,"D"))); - SQLFragment test7 = QueryServiceImpl.get().getSelectSQL(wrapped, - List.of(A,B,C), null, - new Sort("B"), 1000, 0, true); + SQLFragment test7 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,C)).sort(new Sort("B")).buildSqlFragment(); assertFalse(test7.toDebugString().contains("ORDER BY D ASC")); assertTrue(test7.toDebugString().contains("ORDER BY B ASC")); assertTrue(isSorted(test7,2)); diff --git a/query/src/org/labkey/query/controllers/SqlController.java b/query/src/org/labkey/query/controllers/SqlController.java index 2b0995e91e4..189a0230a16 100644 --- a/query/src/org/labkey/query/controllers/SqlController.java +++ b/query/src/org/labkey/query/controllers/SqlController.java @@ -212,7 +212,7 @@ public Object execute(SqlForm form, BindException errors) throws ServletExceptio return null; } - try (Results rs = QueryService.get().selectResults(schema, form.getSql(), null, form.getParameterMap(), true, false)) + try (Results rs = QueryService.get().getSelectBuilder(schema, form.getSql(), true).select(false, form.getParameterMap())) { getViewContext().getResponse().setContentType("text/plain"); if (form.compact) diff --git a/query/src/org/labkey/query/sql/QuerySelectView.java b/query/src/org/labkey/query/sql/QuerySelectView.java index 76eee5c368d..a05bd433d4c 100644 --- a/query/src/org/labkey/query/sql/QuerySelectView.java +++ b/query/src/org/labkey/query/sql/QuerySelectView.java @@ -184,8 +184,7 @@ public Set getSuggestedColumns(Set selected) /* - * This code used to live in QueryService.getSelectSQL(). That code is still public and supported, but eventually - * calls this implementation. + * Called from SelectBuilderImpl.buildSqlFragment() via QuerySelectView.create(). */ private SQLFragment getSelectSQL(TableInfo table, @Nullable Collection selectColumns, @Nullable Filter filter, @Nullable Sort sort, int maxRows, long offset, boolean forceSort, @NotNull QueryLogging queryLogging, boolean distinct) diff --git a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java index 2089c35b338..a26d066e34e 100644 --- a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java +++ b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java @@ -949,7 +949,7 @@ public Map> load(@NotNull Container c, @Nullable Obj TableInfo tableInfo = schema.getTable(SpecimenTablesProvider.SPECIMEN_WRAP_TABLE_NAME); Map columnMap = queryService.getColumns(tableInfo, fieldKeys); - SQLFragment inner = queryService.getSelectSQL(tableInfo, columnMap.values(), null, null, -1, 0, false); + SQLFragment inner = queryService.getSelectBuilder(tableInfo).columns(columnMap.values()).buildSqlFragment(); // Insert COUNT String sampleCountName = tableInfo.getSqlDialect().makeLegalIdentifier("SampleCount"); diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 13c9826c20d..d814c36d97f 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -1370,7 +1370,7 @@ public ActionURL autoLinkResults(ExpProtocol protocol, AssayProvider provider, E final Map keys = new LongHashMap<>(); assert cols.get(runFK) != null : "Could not find object id column: " + objectIdFK; - SQLFragment sql = QueryService.get().getSelectSQL(resultTable, cols.values(), new SimpleFilter(runFK, run.getRowId()), null, Table.ALL_ROWS, Table.NO_OFFSET, false); + SQLFragment sql = QueryService.get().getSelectBuilder(resultTable).columns(cols.values()).filter(new SimpleFilter(runFK, run.getRowId())).buildSqlFragment(); Container targetContainer = targetStudyContainer; new SqlSelector(resultTable.getSchema(), sql).forEach(rs -> { diff --git a/study/src/org/labkey/study/writer/DatasetDataWriter.java b/study/src/org/labkey/study/writer/DatasetDataWriter.java index 1a84aa28704..1d52116f712 100644 --- a/study/src/org/labkey/study/writer/DatasetDataWriter.java +++ b/study/src/org/labkey/study/writer/DatasetDataWriter.java @@ -162,7 +162,7 @@ public void write(StudyImpl study, StudyExportContext ctx, VirtualFile root) thr if (ctx.isDataspaceProject()) DefaultStudyDesignWriter.createExtraForeignKeyColumns(ti, columns); var select = QueryService.get().getSelectBuilder(ti).columns(columns).filter(filter).sort(sort); - ResultsFactory factory = ()->select.select(Map.of(),false); + ResultsFactory factory = ()->select.select(false); writeResultsToTSV(factory, vf, def.getFileName()); } } diff --git a/study/src/org/labkey/study/writer/DefaultStudyDesignWriter.java b/study/src/org/labkey/study/writer/DefaultStudyDesignWriter.java index a3d5083f596..1bb5a745535 100644 --- a/study/src/org/labkey/study/writer/DefaultStudyDesignWriter.java +++ b/study/src/org/labkey/study/writer/DefaultStudyDesignWriter.java @@ -72,7 +72,7 @@ protected void writeTableData(StudyExportContext ctx, VirtualFile vf, TableInfo if (table != null) { var select = QueryService.get().getSelectBuilder(table).columns(columns); - ResultsFactory factory = ()->select.select(Map.of(),false); + ResultsFactory factory = ()->select.select(false); writeResultsToTSV(factory, vf, getFileName(table)); } } diff --git a/visualization/src/org/labkey/visualization/VisualizationController.java b/visualization/src/org/labkey/visualization/VisualizationController.java index 22a566ce07e..3eecc29e77b 100644 --- a/visualization/src/org/labkey/visualization/VisualizationController.java +++ b/visualization/src/org/labkey/visualization/VisualizationController.java @@ -383,7 +383,7 @@ public ApiResponse execute(DimensionValuesForm form, BindException errors) } QueryLogging queryLogging = new QueryLogging(); - SQLFragment sql = QueryService.get().getSelectSQL(tinfo, Collections.singleton(col), filter, null, Table.ALL_ROWS, Table.NO_OFFSET, false, queryLogging); + SQLFragment sql = QueryService.get().getSelectBuilder(tinfo).columns(Collections.singleton(col)).filter(filter).queryLogging(queryLogging).buildSqlFragment(); SQLFragment distinctSql = new SQLFragment(sql); int i = StringUtils.indexOf(sql.getSqlCharSequence(), "SELECT"); if (i >= 0) diff --git a/visualization/src/org/labkey/visualization/sql/VisualizationCDSGenerator.java b/visualization/src/org/labkey/visualization/sql/VisualizationCDSGenerator.java index e92a4d8a116..e9f1b047613 100644 --- a/visualization/src/org/labkey/visualization/sql/VisualizationCDSGenerator.java +++ b/visualization/src/org/labkey/visualization/sql/VisualizationCDSGenerator.java @@ -537,7 +537,7 @@ Results getResults(VisDataRequest q) throws SQLGenerationException, SQLException BindException errors = new NullSafeBindException(q,"query"); String sql = gen.getSQL(errors); assertFalse(errors.hasErrors()); - return QueryService.get().selectResults(schema, sql, null, null, true, true); + return QueryService.get().getSelectBuilder(schema, sql, true).select(); } 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 dd7d491082d..6fdeacb822d 100644 --- a/visualization/src/org/labkey/visualization/sql/VisualizationSQLGenerator.java +++ b/visualization/src/org/labkey/visualization/sql/VisualizationSQLGenerator.java @@ -1140,7 +1140,7 @@ 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().selectResults(schema, sql, null, null, true, true); + return QueryService.get().getSelectBuilder(schema, sql, true).select(); } From 7ab9b7fd754f544f43f36ebb6c2f426693788b59 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 24 May 2026 10:17:18 -0700 Subject: [PATCH 2/2] Fix testGetSelectSqlSort: restore forceSort behavior after API migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test was migrated from getSelectSQL(..., 1000, 0, true) to getSelectBuilder().buildSqlFragment() but lost the forceSort=true flag. Without it, the sort-resolution block (appendDefaultSort, sortFieldKeys resolution) is never entered, causing implicit sorts and B→C sortFieldKey substitutions to be skipped. Co-Authored-By: Claude Sonnet 4.6 --- .../org/labkey/query/QueryServiceImplTestCase.jsp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/query/src/org/labkey/query/QueryServiceImplTestCase.jsp b/query/src/org/labkey/query/QueryServiceImplTestCase.jsp index c9cd240d52e..c9e2c5f57f1 100644 --- a/query/src/org/labkey/query/QueryServiceImplTestCase.jsp +++ b/query/src/org/labkey/query/QueryServiceImplTestCase.jsp @@ -64,27 +64,27 @@ var xyz = wrapped.getColumn("xyz"); // test choose PK - SQLFragment test1 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(C,B,A)).buildSqlFragment(); + SQLFragment test1 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(C,B,A)).forceSort(true).buildSqlFragment(); assertTrue(test1.toDebugString().contains("ORDER BY A ASC")); assertTrue(isSorted(test1,3)); // test choose non-PK - SQLFragment test2 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(C)).buildSqlFragment(); + SQLFragment test2 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(C)).forceSort(true).buildSqlFragment(); assertTrue(test2.toDebugString().contains("ORDER BY C ASC")); assertTrue(isSorted(test2,1)); // test explicit in select list - SQLFragment test3 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,C)).sort(new Sort("C")).buildSqlFragment(); + SQLFragment test3 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,C)).sort(new Sort("C")).forceSort(true).buildSqlFragment(); assertTrue(test3.toDebugString().contains("ORDER BY C ASC")); assertTrue(isSorted(test3,2)); // test explicit not in select list - SQLFragment test4 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,xyz)).sort(new Sort("C")).buildSqlFragment(); + SQLFragment test4 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,xyz)).sort(new Sort("C")).forceSort(true).buildSqlFragment(); assertTrue(test4.toDebugString().contains("ORDER BY C ASC")); assertTrue(isSorted(test4,3)); // test sortFieldKeys - SQLFragment test5 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,xyz)).sort(new Sort("B")).buildSqlFragment(); + SQLFragment test5 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,xyz)).sort(new Sort("B")).forceSort(true).buildSqlFragment(); assertFalse(test5.toDebugString().contains("ORDER BY B ASC")); assertTrue(test5.toDebugString().contains("ORDER BY C ASC")); assertTrue(isSorted(test5,3)); @@ -93,14 +93,14 @@ (wrapped.getMutableColumn("B")).setSortFieldKeys(Arrays.asList(new FieldKey(null,"D"))); // implicit sort, sort by B because D not found - SQLFragment test6 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(B)).buildSqlFragment(); + SQLFragment test6 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(B)).forceSort(true).buildSqlFragment(); assertFalse(test6.toDebugString().contains("ORDER BY D ASC")); assertTrue(test6.toDebugString().contains("ORDER BY B ASC")); assertTrue(isSorted(test6,1)); // explicit sort, sort by B because D not found (wrapped.getMutableColumn("B")).setSortFieldKeys(Arrays.asList(new FieldKey(null,"D"))); - SQLFragment test7 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,C)).sort(new Sort("B")).buildSqlFragment(); + SQLFragment test7 = QueryServiceImpl.get().getSelectBuilder(wrapped).columns(List.of(A,B,C)).sort(new Sort("B")).forceSort(true).buildSqlFragment(); assertFalse(test7.toDebugString().contains("ORDER BY D ASC")); assertTrue(test7.toDebugString().contains("ORDER BY B ASC")); assertTrue(isSorted(test7,2));