From 828fcb0beaa3e9636e5ac689245a176949025572 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Thu, 9 Jul 2026 23:05:54 -0600 Subject: [PATCH 01/14] Fix isnumeric() in boolean context on PostgreSQL --- .../org/labkey/api/data/dialect/BasePostgreSqlDialect.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index c2a43f9747a..e62db0a913e 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -937,8 +937,11 @@ public boolean supportsIsNumeric() @Override public SQLFragment isNumericExpr(SQLFragment expression) { - return new SQLFragment("(CASE WHEN CAST((").append(expression) - .append(") AS TEXT) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)$' THEN 1 ELSE 0 END)"); + // Return a boolean predicate, matching SQL Server's contract, so callers that place this in a + // boolean context (CASE WHEN, WHERE) get valid Postgres syntax. In SELECT position Postgres + // returns it as a boolean column and JDBC's getInt() converts true/false to 1/0. + return new SQLFragment("(CAST((").append(expression) + .append(") AS TEXT) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)$')"); } private class PostgreSqlColumnMetaDataReader extends ColumnMetaDataReader From 57d911613cf21e47244f8080c4c47b9cef4d3c23 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Wed, 15 Jul 2026 21:50:47 -0600 Subject: [PATCH 02/14] Fix Pivot: bind pivot values as typed parameters Co-Authored-By: Claude Opus 4.7 --- query/src/org/labkey/query/sql/QueryPivot.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/query/src/org/labkey/query/sql/QueryPivot.java b/query/src/org/labkey/query/sql/QueryPivot.java index 42175beebcc..79cbadb2db8 100644 --- a/query/src/org/labkey/query/sql/QueryPivot.java +++ b/query/src/org/labkey/query/sql/QueryPivot.java @@ -549,7 +549,9 @@ public Map getAllColumns() String pivotName = makePivotAggName(name, pivotValue); RelationColumn pvt = _makePivotedAggColumn(s, new FieldKey(null, pivotName), pivotValue); - _columns.put(pivotName, pvt); + // _makePivotedAggColumn() returns null when parse errors are present; don't store nulls + if (null != pvt) + _columns.put(pivotName, pvt); } } } @@ -822,9 +824,20 @@ public SQLFragment getSql() String alias = makePivotColumnAlias(col.getAlias(), pivotValue.getKey()); sql.append(comma).append("MAX(CASE WHEN (").append(_pivotColumn.getValueSql()); if (value instanceof QNull) + { sql.append(" IS NULL"); + } else - sql.append("=").append(value.getSourceText()); + { + // Bind the pivot value as a parameter rather than embedding its source-text literal. + // Embedding via getSourceText() trips SQLFragment's semicolon/quote guardrail when the + // value contains ';' or unbalanced '"' (both legal inside SQL string literals, but the + // guardrail can't distinguish them from unsafe raw SQL). + // Bind with an explicit JdbcType so Postgres can resolve the parameter's type — an + // untyped bind can trip "could not determine data type of parameter $N" in some plans. + sql.append("=?"); + sql.add(((IConstant) value).getValue(), ((QExpr) value).getJdbcType()); + } sql.append(") THEN (").append(col.getValueSql()).append(") ELSE NULL END) AS ").appendIdentifier(alias); comma = ",\n"; } From 93b0fc55c8e4965811354cf238e1dd14490e5c7b Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Sun, 26 Jul 2026 21:44:14 -0600 Subject: [PATCH 03/14] Make LabKey SQL week() match SQL Server numbering on PostgreSQL --- .../data/dialect/BasePostgreSqlDialect.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index e62db0a913e..fe473aff3a5 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -1133,6 +1133,8 @@ public SQLFragment formatJdbcFunction(String fn, SQLFragment... arguments) return formatFunction(call, nativeFn, arguments); else if (fn.equalsIgnoreCase("timestampdiff")) return timestampdiff(arguments); + else if (fn.equalsIgnoreCase("week")) + return week(arguments); else return super.formatJdbcFunction(fn, arguments); } @@ -1173,6 +1175,24 @@ private SQLFragment timestampdiff(SQLFragment... arguments) return super.formatJdbcFunction("timestampdiff", arguments); } + /* week() inconsistent between sql server and postgres: pgjdbc translates {fn week(x)} to + * EXTRACT(WEEK FROM x), which returns ISO 8601 week numbering (week 1 contains the year's + * first Thursday; weeks start on Monday). The Microsoft SQL Server JDBC driver translates + * {fn week(x)} to DATEPART(week, x), which uses US-style numbering (week 1 always contains + * Jan 1; weeks start on Sunday under the default DATEFIRST=7). The two agree most of the + * year but disagree by 1 on Sundays and around year boundaries. Emit an equivalent US-style + * expression here so LabKey SQL's week() returns matching values on both databases. + */ + private SQLFragment week(SQLFragment... arguments) + { + SQLFragment ret = new SQLFragment("CAST(FLOOR((EXTRACT(doy FROM "); + ret.append(arguments[0]); + ret.append(") + EXTRACT(dow FROM date_trunc('year', "); + ret.append(arguments[0]); + ret.append(")) - 1) / 7) + 1 AS INTEGER)"); + return ret; + } + @Override public boolean supportsBatchGeneratedKeys() { From a4bc7efcd37d694eae6b94ae795960da724005ce Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Thu, 27 Aug 2026 11:03:03 -0600 Subject: [PATCH 04/14] Bind pivot values using the pivot column's type QueryPivot.getSql() bound each pivot value using the constant's own JdbcType, but wrapConstant() never produces QDate/QTimestamp -- a date or timestamp pivot column arrives here as a QString, so the parameter was bound as VARCHAR and Postgres rejected the comparison with "operator does not exist: timestamp without time zone = character varying". SQL Server converts implicitly, so only Postgres was affected. Prefer the pivot column's own type and convert the value to it, falling back to the constant's type and value when that conversion doesn't hold, since the IN (SELECT ...) form can supply values from an expression other than the pivot column. Where the two types already match -- every string pivot -- the emitted SQL and bound value are unchanged. --- .../src/org/labkey/query/sql/QueryPivot.java | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/query/src/org/labkey/query/sql/QueryPivot.java b/query/src/org/labkey/query/sql/QueryPivot.java index 79cbadb2db8..95f9a6fb974 100644 --- a/query/src/org/labkey/query/sql/QueryPivot.java +++ b/query/src/org/labkey/query/sql/QueryPivot.java @@ -15,6 +15,7 @@ */ package org.labkey.query.sql; +import org.apache.commons.beanutils.ConversionException; import org.apache.commons.beanutils.ConvertUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -829,14 +830,29 @@ public SQLFragment getSql() } else { - // Bind the pivot value as a parameter rather than embedding its source-text literal. - // Embedding via getSourceText() trips SQLFragment's semicolon/quote guardrail when the - // value contains ';' or unbalanced '"' (both legal inside SQL string literals, but the - // guardrail can't distinguish them from unsafe raw SQL). - // Bind with an explicit JdbcType so Postgres can resolve the parameter's type — an - // untyped bind can trip "could not determine data type of parameter $N" in some plans. + // Bind the pivot value as a parameter instead of embedding it directly in the SQL. + // This safely handles values containing characters like ';' or quotes. + // + // Use an explicit JdbcType so Postgres can determine the parameter type. + // Prefer the pivot column's type, especially for date/timestamp columns, to avoid + // type mismatch errors. Fall back to the constant's type if conversion isn't possible. + Object bindValue = ((IConstant) value).getValue(); + JdbcType bindType = ((QExpr) value).getJdbcType(); + JdbcType columnType = _pivotColumn.getJdbcType(); + if (null != columnType && JdbcType.OTHER != columnType && columnType != bindType) + { + try + { + bindValue = columnType.convert(bindValue); + bindType = columnType; + } + catch (ConversionException ignored) + { + // keep the constant's own type and value + } + } sql.append("=?"); - sql.add(((IConstant) value).getValue(), ((QExpr) value).getJdbcType()); + sql.add(bindValue, bindType); } sql.append(") THEN (").append(col.getValueSql()).append(") ELSE NULL END) AS ").appendIdentifier(alias); comma = ",\n"; From e82602f19fa69d6302b47379f4d9f92ffc8d3a6d Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Thu, 27 Aug 2026 11:03:20 -0600 Subject: [PATCH 05/14] Surface pivot parse errors instead of dropping pivoted columns _makePivotedAggColumn() returns null whenever parse errors are present, so QueryPivot.getAllColumns() silently omitted every :: column. The TableInfo's column set is built from getAllColumns() through initializeColumns(), and that path never reaches getSql(), so a failure such as "Pivot query unauthorized" produced a successfully-constructed table missing all of its pivoted columns with no error raised anywhere -- the schema browser, getQueryDetails and the custom view field picker all show the query as simply having no pivoted output. Throw the underlying parse error the way getSql() and getColMembers() already do, and discard the partially-built _columns map first so a later call re-derives it rather than serving the short list from cache. Only fires when a column was actually dropped, so the complete-column path is unaffected. --- query/src/org/labkey/query/sql/QueryPivot.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/query/src/org/labkey/query/sql/QueryPivot.java b/query/src/org/labkey/query/sql/QueryPivot.java index 95f9a6fb974..d5742d4f772 100644 --- a/query/src/org/labkey/query/sql/QueryPivot.java +++ b/query/src/org/labkey/query/sql/QueryPivot.java @@ -539,6 +539,7 @@ public Map getAllColumns() } // Add the pivoted aggregate columns grouped by pivot value + boolean droppedColumn = false; if (!aggs.isEmpty()) { for (String pivotValue : pivotValues.keySet()) @@ -553,9 +554,23 @@ public Map getAllColumns() // _makePivotedAggColumn() returns null when parse errors are present; don't store nulls if (null != pvt) _columns.put(pivotName, pvt); + else + droppedColumn = true; } } } + + // Returning a silently short column list is harder to diagnose than the error that caused it, so + // surface the underlying parse error the way getSql() and getColMembers() do. Only fires when a + // column was actually dropped, so the complete-column path is unaffected. _columns is discarded + // first: it is cached, and a partial map would be handed out unguarded on any subsequent call. + if (droppedColumn && !getParseErrors().isEmpty()) + { + _columns = null; + QueryException qe = getParseErrors().get(0); + _query.decorateException(qe); + throw qe; + } } return _columns; } From f3f6e0e8b26489851ca336759e29ab7d9578c0c0 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Thu, 27 Aug 2026 11:03:42 -0600 Subject: [PATCH 06/14] Correct stale isnumeric() comments --- query/src/org/labkey/query/QueryServiceImpl.java | 2 +- query/src/org/labkey/query/sql/Method.java | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index c83c5b95972..211d7b14a0d 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3787,7 +3787,7 @@ public void testWhereClauseWithUnion() public void testRightAndIsnumeric() throws SQLException { // Portable LabKey-SQL functions: right() dispatches via the JDBC {fn right} escape; - // isnumeric() emits ISNUMERIC(x) on SQL Server and a regex-based CASE on PostgreSQL. + // isnumeric() returns a boolean predicate on both -- (ISNUMERIC(x) = 1) on SQL Server, a regex match on PostgreSQL. // This test exercises both against whichever dialect the test container is using. String sql = "SELECT " + diff --git a/query/src/org/labkey/query/sql/Method.java b/query/src/org/labkey/query/sql/Method.java index f4829cc4a11..47ed86f2fef 100644 --- a/query/src/org/labkey/query/sql/Method.java +++ b/query/src/org/labkey/query/sql/Method.java @@ -1076,9 +1076,11 @@ public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) } } - // Portable isnumeric() emits ISNUMERIC(x) on SQL Server and a regex-based CASE on PostgreSQL. - // Returns 1 for digit strings with an optional sign/decimal point, 0 otherwise. - // This is stricter than SQL Server's ISNUMERIC(), which also accepts formats like scientific notation. + // Portable isnumeric() returns a boolean predicate on both databases, so it is valid in a boolean + // context (CASE WHEN, WHERE) as well as in a SELECT list: (ISNUMERIC(x) = 1) on SQL Server, and a + // regex match on PostgreSQL. True for digit strings with an optional sign/decimal point. + // The PostgreSQL regex is stricter than SQL Server's ISNUMERIC(), which also accepts formats like + // scientific notation and currency. static class IsNumericInfo extends AbstractMethodInfo { IsNumericInfo() From c08c994027421479d2ff09bd5c6e09c16cf92cfb Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Wed, 2 Sep 2026 10:53:04 -0600 Subject: [PATCH 07/14] Trim comments on the Postgres dialect and PIVOT changes --- .../data/dialect/BasePostgreSqlDialect.java | 14 +++----------- .../src/org/labkey/query/QueryServiceImpl.java | 5 ++--- query/src/org/labkey/query/sql/Method.java | 8 +++----- query/src/org/labkey/query/sql/QueryPivot.java | 18 +++++++----------- 4 files changed, 15 insertions(+), 30 deletions(-) diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index fe473aff3a5..373d64099e3 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -937,9 +937,7 @@ public boolean supportsIsNumeric() @Override public SQLFragment isNumericExpr(SQLFragment expression) { - // Return a boolean predicate, matching SQL Server's contract, so callers that place this in a - // boolean context (CASE WHEN, WHERE) get valid Postgres syntax. In SELECT position Postgres - // returns it as a boolean column and JDBC's getInt() converts true/false to 1/0. + // A boolean predicate, not 1/0, to match SQL Server's contract; in SELECT position JDBC's getInt() converts true/false to 1/0. return new SQLFragment("(CAST((").append(expression) .append(") AS TEXT) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)$')"); } @@ -1175,14 +1173,8 @@ private SQLFragment timestampdiff(SQLFragment... arguments) return super.formatJdbcFunction("timestampdiff", arguments); } - /* week() inconsistent between sql server and postgres: pgjdbc translates {fn week(x)} to - * EXTRACT(WEEK FROM x), which returns ISO 8601 week numbering (week 1 contains the year's - * first Thursday; weeks start on Monday). The Microsoft SQL Server JDBC driver translates - * {fn week(x)} to DATEPART(week, x), which uses US-style numbering (week 1 always contains - * Jan 1; weeks start on Sunday under the default DATEFIRST=7). The two agree most of the - * year but disagree by 1 on Sundays and around year boundaries. Emit an equivalent US-style - * expression here so LabKey SQL's week() returns matching values on both databases. - */ + // pgjdbc translates {fn week(x)} to EXTRACT(WEEK FROM x) -- ISO 8601, weeks start Monday -- while the SQL Server + // driver emits DATEPART(week, x) -- US-style, weeks start Sunday. Emit US-style so both databases agree. private SQLFragment week(SQLFragment... arguments) { SQLFragment ret = new SQLFragment("CAST(FLOOR((EXTRACT(doy FROM "); diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 211d7b14a0d..8efc080ab25 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3786,9 +3786,8 @@ public void testWhereClauseWithUnion() @Test public void testRightAndIsnumeric() throws SQLException { - // Portable LabKey-SQL functions: right() dispatches via the JDBC {fn right} escape; - // isnumeric() returns a boolean predicate on both -- (ISNUMERIC(x) = 1) on SQL Server, a regex match on PostgreSQL. - // This test exercises both against whichever dialect the test container is using. + // Portable LabKey-SQL functions: right() dispatches via the JDBC {fn right} escape; isnumeric() is a + // boolean predicate on both -- (ISNUMERIC(x) = 1) on SQL Server, a regex match on PostgreSQL. String sql = "SELECT " + " right('hello', 2) AS r1, " + diff --git a/query/src/org/labkey/query/sql/Method.java b/query/src/org/labkey/query/sql/Method.java index 47ed86f2fef..3ed34d568ed 100644 --- a/query/src/org/labkey/query/sql/Method.java +++ b/query/src/org/labkey/query/sql/Method.java @@ -1076,11 +1076,9 @@ public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) } } - // Portable isnumeric() returns a boolean predicate on both databases, so it is valid in a boolean - // context (CASE WHEN, WHERE) as well as in a SELECT list: (ISNUMERIC(x) = 1) on SQL Server, and a - // regex match on PostgreSQL. True for digit strings with an optional sign/decimal point. - // The PostgreSQL regex is stricter than SQL Server's ISNUMERIC(), which also accepts formats like - // scientific notation and currency. + // Portable isnumeric() is a boolean predicate on both databases -- (ISNUMERIC(x) = 1) on SQL Server, a regex + // match on PostgreSQL -- so it is valid in CASE WHEN and WHERE, not just a SELECT list. The PostgreSQL regex + // accepts only digits with an optional sign/decimal point, stricter than SQL Server's ISNUMERIC(). static class IsNumericInfo extends AbstractMethodInfo { IsNumericInfo() diff --git a/query/src/org/labkey/query/sql/QueryPivot.java b/query/src/org/labkey/query/sql/QueryPivot.java index d5742d4f772..0075b13e0c7 100644 --- a/query/src/org/labkey/query/sql/QueryPivot.java +++ b/query/src/org/labkey/query/sql/QueryPivot.java @@ -551,7 +551,7 @@ public Map getAllColumns() String pivotName = makePivotAggName(name, pivotValue); RelationColumn pvt = _makePivotedAggColumn(s, new FieldKey(null, pivotName), pivotValue); - // _makePivotedAggColumn() returns null when parse errors are present; don't store nulls + // _makePivotedAggColumn() returns null when parse errors are present if (null != pvt) _columns.put(pivotName, pvt); else @@ -560,10 +560,9 @@ public Map getAllColumns() } } - // Returning a silently short column list is harder to diagnose than the error that caused it, so - // surface the underlying parse error the way getSql() and getColMembers() do. Only fires when a - // column was actually dropped, so the complete-column path is unaffected. _columns is discarded - // first: it is cached, and a partial map would be handed out unguarded on any subsequent call. + // A silently short column list is harder to diagnose than the parse error behind it, so throw the way + // getSql() and getColMembers() do. Discard the cached _columns first, or the partial map gets handed + // out unguarded on the next call. if (droppedColumn && !getParseErrors().isEmpty()) { _columns = null; @@ -845,12 +844,9 @@ public SQLFragment getSql() } else { - // Bind the pivot value as a parameter instead of embedding it directly in the SQL. - // This safely handles values containing characters like ';' or quotes. - // - // Use an explicit JdbcType so Postgres can determine the parameter type. - // Prefer the pivot column's type, especially for date/timestamp columns, to avoid - // type mismatch errors. Fall back to the constant's type if conversion isn't possible. + // Bind rather than embed the source text: a value containing ';' or a quote trips SQLFragment's guardrail. + // Postgres needs an explicit parameter type, and wrapConstant() types date/timestamp pivot values as + // QString, so prefer the pivot column's type and fall back to the constant's if it won't convert. Object bindValue = ((IConstant) value).getValue(); JdbcType bindType = ((QExpr) value).getJdbcType(); JdbcType columnType = _pivotColumn.getJdbcType(); From 708b8b23eafb52d635df81e03fc3ecaf4ec7b697 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Fri, 4 Sep 2026 12:43:10 -0600 Subject: [PATCH 08/14] Add integration test for LabKey SQL week() numbering Covers the Postgres dialect override that emits US-style week numbers. The dates are chosen to exercise both divergent rules and the year boundary; a mid-year sample in a Mon-Thu year passes with the bug fully present. --- .../org/labkey/query/QueryServiceImpl.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 8efc080ab25..e607d35ed03 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3820,5 +3820,47 @@ public void testRightAndIsnumeric() throws SQLException assertEquals("isnumeric(NULL) on " + dialect, 0, results.getInt("n4")); } } + + @Test + public void testWeek() throws SQLException + { + // week() must return SQL Server's US numbering on both platforms: weeks start Sunday, and week 1 is + // whatever week contains Jan 1. pgjdbc expands {fn week} to ISO 8601 -- Monday start, week 1 anchored + // on the year's first Thursday -- so BasePostgreSqlDialect intercepts it rather than deferring. + // + // The two rules diverge independently, and how they combine depends on the day Jan 1 falls on, so a + // single year badly understates the difference. 2026 (Jan 1 = Thursday) agrees with ISO except on + // Sundays; 2027 (Jan 1 = Friday) is off by one every day, and ISO assigns 2027-01-01 to week 53 of + // the prior year. Both are covered below; do not reduce this to a mid-year sample. + String sql = + "SELECT " + + " week(CAST('2026-01-01 00:00:00' AS TIMESTAMP)) AS w1, " + // Thursday -> 1 + " week(CAST('2026-01-03 00:00:00' AS TIMESTAMP)) AS w2, " + // Saturday -> 1 + " week(CAST('2026-01-04 00:00:00' AS TIMESTAMP)) AS w3, " + // Sunday -> 2 (ISO gives 1) + " week(CAST('2027-01-01 00:00:00' AS TIMESTAMP)) AS w4, " + // Friday -> 1 (ISO gives 53) + " week(CAST('2027-07-15 00:00:00' AS TIMESTAMP)) AS w5 " + // Thursday -> 29 (ISO gives 28) + "FROM core.Containers"; + + QueryDef qd = new QueryDef(); + qd.setSchema("core"); + qd.setName("junit" + GUID.makeHash()); + qd.setContainer(JunitUtil.getTestContainer().getId()); + qd.setSql(sql); + QueryDefinition qdef = new CustomQueryDefinitionImpl(TestContext.get().getUser(), JunitUtil.getTestContainer(), qd); + List errors = new ArrayList<>(); + TableInfo t = qdef.getTable(errors, false); + String dialect = t == null ? "?" : t.getSqlDialect().getProductName(); + assertTrue("Query parse errors on " + dialect + ": " + errors, errors.isEmpty()); + + try (Results results = new TableSelector(t).getResults()) + { + assertTrue("Expected at least one row from core.Containers", results.next()); + assertEquals("week(2026-01-01), Thursday, on " + dialect, 1, results.getInt("w1")); + assertEquals("week(2026-01-03), Saturday, on " + dialect, 1, results.getInt("w2")); + assertEquals("week(2026-01-04), Sunday, on " + dialect, 2, results.getInt("w3")); + assertEquals("week(2027-01-01), Friday, on " + dialect, 1, results.getInt("w4")); + assertEquals("week(2027-07-15), Fri-Sun-anchored year, on " + dialect, 29, results.getInt("w5")); + } + } } } From 67de1dad9c669639f1773bb25c5c954a25d3b6bc Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Tue, 8 Sep 2026 10:59:35 -0600 Subject: [PATCH 09/14] Clarify testWeek comment --- query/src/org/labkey/query/QueryServiceImpl.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index e607d35ed03..569b7dbbce3 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3824,14 +3824,15 @@ public void testRightAndIsnumeric() throws SQLException @Test public void testWeek() throws SQLException { - // week() must return SQL Server's US numbering on both platforms: weeks start Sunday, and week 1 is - // whatever week contains Jan 1. pgjdbc expands {fn week} to ISO 8601 -- Monday start, week 1 anchored - // on the year's first Thursday -- so BasePostgreSqlDialect intercepts it rather than deferring. + // Verifies week() returns the same number on both platforms. // - // The two rules diverge independently, and how they combine depends on the day Jan 1 falls on, so a - // single year badly understates the difference. 2026 (Jan 1 = Thursday) agrees with ISO except on - // Sundays; 2027 (Jan 1 = Friday) is off by one every day, and ISO assigns 2027-01-01 to week 53 of - // the prior year. Both are covered below; do not reduce this to a mid-year sample. + // pgjdbc expands {fn week} to ISO 8601 numbering (weeks start Monday, week 1 holds the year's first + // Thursday) where SQL Server uses US numbering (weeks start Sunday, week 1 holds Jan 1), so the two + // disagreed. BasePostgreSqlDialect.formatJdbcFunction now emits the US form instead of deferring to + // the driver. + // + // The dates cover both rule differences and the year boundary; a mid-year sample in a year whose + // Jan 1 falls Mon-Thu passes either way. String sql = "SELECT " + " week(CAST('2026-01-01 00:00:00' AS TIMESTAMP)) AS w1, " + // Thursday -> 1 From d3e6a078b0a9fae25865c43caec34d20e8fe1568 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Mon, 14 Sep 2026 12:37:31 -0600 Subject: [PATCH 10/14] Revert the Postgres week() interception and add weekus()/weekiso() week() is a passthrough, so overriding it on Postgres moved WeekOfYear for every existing caller, including the shared EHR and LDK date range lookups. The two new functions are defined by LabKey and return the same number on either dialect, so callers pick a numbering instead of inheriting the driver's. --- .../data/dialect/BasePostgreSqlDialect.java | 29 +++++----- .../labkey/api/data/dialect/SqlDialect.java | 18 ++++++ .../org/labkey/query/QueryServiceImpl.java | 58 +++++++++++-------- query/src/org/labkey/query/sql/Method.java | 47 +++++++++++++++ 4 files changed, 113 insertions(+), 39 deletions(-) diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index 373d64099e3..ede9d656c29 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -942,6 +942,21 @@ public SQLFragment isNumericExpr(SQLFragment expression) .append(") AS TEXT) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)$')"); } + @Override + public SQLFragment weekIsoExpr(SQLFragment expression) + { + return new SQLFragment("CAST(EXTRACT(week FROM (").append(expression).append(")) AS INTEGER)"); + } + + @Override + public SQLFragment weekUsExpr(SQLFragment expression) + { + // Week 1 is whatever week contains Jan 1, and weeks start Sunday: (day of year + Sunday-based weekday of Jan 1 - 1) / 7, rounded down, plus one. + return new SQLFragment("CAST(FLOOR((EXTRACT(doy FROM (").append(expression) + .append(")) + EXTRACT(dow FROM date_trunc('year', (").append(expression) + .append("))) - 1) / 7) + 1 AS INTEGER)"); + } + private class PostgreSqlColumnMetaDataReader extends ColumnMetaDataReader { private final TableInfo _table; @@ -1131,8 +1146,6 @@ public SQLFragment formatJdbcFunction(String fn, SQLFragment... arguments) return formatFunction(call, nativeFn, arguments); else if (fn.equalsIgnoreCase("timestampdiff")) return timestampdiff(arguments); - else if (fn.equalsIgnoreCase("week")) - return week(arguments); else return super.formatJdbcFunction(fn, arguments); } @@ -1173,18 +1186,6 @@ private SQLFragment timestampdiff(SQLFragment... arguments) return super.formatJdbcFunction("timestampdiff", arguments); } - // pgjdbc translates {fn week(x)} to EXTRACT(WEEK FROM x) -- ISO 8601, weeks start Monday -- while the SQL Server - // driver emits DATEPART(week, x) -- US-style, weeks start Sunday. Emit US-style so both databases agree. - private SQLFragment week(SQLFragment... arguments) - { - SQLFragment ret = new SQLFragment("CAST(FLOOR((EXTRACT(doy FROM "); - ret.append(arguments[0]); - ret.append(") + EXTRACT(dow FROM date_trunc('year', "); - ret.append(arguments[0]); - ret.append(")) - 1) / 7) + 1 AS INTEGER)"); - return ret; - } - @Override public boolean supportsBatchGeneratedKeys() { diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index a7dc67defef..52878030c0a 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SqlDialect.java @@ -847,6 +847,24 @@ public SQLFragment isNumericExpr(SQLFragment expression) throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement"); } + /** + * ISO 8601 week number, 1 to 53: weeks start Monday and week 1 is the week holding the year's first Thursday. + * A week belongs to whichever year owns its Thursday, so Jan 1-3 can number as week 52 or 53 of the prior year (2027-01-01 is week 53) and Dec 29-31 as week 1 of the next. + */ + public SQLFragment weekIsoExpr(SQLFragment expression) + { + throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement"); + } + + /** + * US week number, 1 to 54: weeks start Sunday and week 1 is whatever week holds Jan 1, so the first and last weeks of a year are both partial. + * Every date numbers within its own calendar year, so Jan 1 is always week 1. Matches SQL Server's DATEPART(week, x) under the default DATEFIRST 7. + */ + public SQLFragment weekUsExpr(SQLFragment expression) + { + throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement"); + } + public void handleCreateDatabaseException(SQLException e) throws ServletException { throw(new ServletException("Can't create database", e)); diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 569b7dbbce3..a24b8b3af4a 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3821,32 +3821,43 @@ public void testRightAndIsnumeric() throws SQLException } } + // 2026 (Jan 1 = Thursday) makes the two rules agree except on Sundays; 2027 (Jan 1 = Friday) puts them one + // apart every day and starts in the prior ISO year. A mid-year sample in a Mon-Thu year passes either way. + private static final String[] WEEK_DATES = { + "2026-01-01", // Thursday + "2026-01-03", // Saturday + "2026-01-04", // Sunday + "2027-01-01", // Friday + "2027-07-15" // Thursday + }; + @Test - public void testWeek() throws SQLException - { - // Verifies week() returns the same number on both platforms. - // - // pgjdbc expands {fn week} to ISO 8601 numbering (weeks start Monday, week 1 holds the year's first - // Thursday) where SQL Server uses US numbering (weeks start Sunday, week 1 holds Jan 1), so the two - // disagreed. BasePostgreSqlDialect.formatJdbcFunction now emits the US form instead of deferring to - // the driver. - // - // The dates cover both rule differences and the year boundary; a mid-year sample in a year whose - // Jan 1 falls Mon-Thu passes either way. - String sql = - "SELECT " + - " week(CAST('2026-01-01 00:00:00' AS TIMESTAMP)) AS w1, " + // Thursday -> 1 - " week(CAST('2026-01-03 00:00:00' AS TIMESTAMP)) AS w2, " + // Saturday -> 1 - " week(CAST('2026-01-04 00:00:00' AS TIMESTAMP)) AS w3, " + // Sunday -> 2 (ISO gives 1) - " week(CAST('2027-01-01 00:00:00' AS TIMESTAMP)) AS w4, " + // Friday -> 1 (ISO gives 53) - " week(CAST('2027-07-15 00:00:00' AS TIMESTAMP)) AS w5 " + // Thursday -> 29 (ISO gives 28) - "FROM core.Containers"; + public void testWeekUs() throws SQLException + { + // Weeks start Sunday and week 1 holds Jan 1, matching SQL Server's DATEPART(week, x) under DATEFIRST 7. + assertWeeks("weekus", 1, 1, 2, 1, 29); + } + + @Test + public void testWeekIso() throws SQLException + { + // ISO 8601: weeks start Monday and week 1 holds the year's first Thursday, so 2027-01-01 lands in 2026's week 53. + assertWeeks("weekiso", 1, 1, 1, 53, 28); + } + + private void assertWeeks(String method, int... expected) throws SQLException + { + StringBuilder sql = new StringBuilder("SELECT "); + for (int i = 0; i < WEEK_DATES.length; i++) + sql.append(i > 0 ? ", " : "").append(method) + .append("(CAST('").append(WEEK_DATES[i]).append(" 00:00:00' AS TIMESTAMP)) AS w").append(i + 1); + sql.append(" FROM core.Containers"); QueryDef qd = new QueryDef(); qd.setSchema("core"); qd.setName("junit" + GUID.makeHash()); qd.setContainer(JunitUtil.getTestContainer().getId()); - qd.setSql(sql); + qd.setSql(sql.toString()); QueryDefinition qdef = new CustomQueryDefinitionImpl(TestContext.get().getUser(), JunitUtil.getTestContainer(), qd); List errors = new ArrayList<>(); TableInfo t = qdef.getTable(errors, false); @@ -3856,11 +3867,8 @@ public void testWeek() throws SQLException try (Results results = new TableSelector(t).getResults()) { assertTrue("Expected at least one row from core.Containers", results.next()); - assertEquals("week(2026-01-01), Thursday, on " + dialect, 1, results.getInt("w1")); - assertEquals("week(2026-01-03), Saturday, on " + dialect, 1, results.getInt("w2")); - assertEquals("week(2026-01-04), Sunday, on " + dialect, 2, results.getInt("w3")); - assertEquals("week(2027-01-01), Friday, on " + dialect, 1, results.getInt("w4")); - assertEquals("week(2027-07-15), Fri-Sun-anchored year, on " + dialect, 29, results.getInt("w5")); + for (int i = 0; i < expected.length; i++) + assertEquals(method + "(" + WEEK_DATES[i] + ") on " + dialect, expected[i], results.getInt("w" + (i + 1))); } } } diff --git a/query/src/org/labkey/query/sql/Method.java b/query/src/org/labkey/query/sql/Method.java index 3ed34d568ed..df9a4c2b735 100644 --- a/query/src/org/labkey/query/sql/Method.java +++ b/query/src/org/labkey/query/sql/Method.java @@ -482,6 +482,25 @@ public MethodInfo getMethodInfo() // Put new methods below this line and move above after they're documented, i.e., // added to https://www.labkey.org/Documentation/wiki-page.view?name=labkeySql + // week() is a passthrough, so its numbering is whatever the database does. These two are defined by LabKey and return the same number on either dialect. + // weekiso(x) -- ISO 8601, 1 to 53. Weeks start Monday and week 1 holds the year's first Thursday, so early January can number as the prior year's week 52 or 53. + // weekus(x) -- US, 1 to 54. Weeks start Sunday and week 1 holds Jan 1, so every date numbers within its own year. + labkeyMethod.put("weekiso", new Method("weekiso", JdbcType.INTEGER, 1, 1) + { + @Override + public MethodInfo getMethodInfo() + { + return new WeekIsoInfo(); + } + }); + labkeyMethod.put("weekus", new Method("weekus", JdbcType.INTEGER, 1, 1) + { + @Override + public MethodInfo getMethodInfo() + { + return new WeekUsInfo(); + } + }); // ========== Don't document these ========== labkeyMethod.put("__cte_two__", new Method(JdbcType.INTEGER, 0, 0) @@ -1097,6 +1116,34 @@ public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) } } + static class WeekIsoInfo extends AbstractMethodInfo + { + WeekIsoInfo() + { + super(JdbcType.INTEGER); + } + + @Override + public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) + { + return dialect.weekIsoExpr(arguments[0]); + } + } + + static class WeekUsInfo extends AbstractMethodInfo + { + WeekUsInfo() + { + super(JdbcType.INTEGER); + } + + @Override + public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) + { + return dialect.weekUsExpr(arguments[0]); + } + } + static class VersionMethodInfo extends AbstractMethodInfo { VersionMethodInfo() From 7d26e5eb557ad292db4f45c9f8acbb5f70f2dea3 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Tue, 15 Sep 2026 12:59:38 -0600 Subject: [PATCH 11/14] Revert "Surface pivot parse errors instead of dropping pivoted columns" This is diagnostics rather than a correctness fix, and it changes behavior every TableInfo runs through, so it moves to its own PR along with the AbstractTableInfo init-guard fix it needs to be safe. --- query/src/org/labkey/query/sql/QueryPivot.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/query/src/org/labkey/query/sql/QueryPivot.java b/query/src/org/labkey/query/sql/QueryPivot.java index 0075b13e0c7..293211983eb 100644 --- a/query/src/org/labkey/query/sql/QueryPivot.java +++ b/query/src/org/labkey/query/sql/QueryPivot.java @@ -539,7 +539,6 @@ public Map getAllColumns() } // Add the pivoted aggregate columns grouped by pivot value - boolean droppedColumn = false; if (!aggs.isEmpty()) { for (String pivotValue : pivotValues.keySet()) @@ -554,22 +553,9 @@ public Map getAllColumns() // _makePivotedAggColumn() returns null when parse errors are present if (null != pvt) _columns.put(pivotName, pvt); - else - droppedColumn = true; } } } - - // A silently short column list is harder to diagnose than the parse error behind it, so throw the way - // getSql() and getColMembers() do. Discard the cached _columns first, or the partial map gets handed - // out unguarded on the next call. - if (droppedColumn && !getParseErrors().isEmpty()) - { - _columns = null; - QueryException qe = getParseErrors().get(0); - _query.decorateException(qe); - throw qe; - } } return _columns; } From 446336787a86447a80bf154bb8e66c9926b654fd Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Tue, 15 Sep 2026 13:00:05 -0600 Subject: [PATCH 12/14] Restore isnumeric() to 1/0 on both databases isnumeric() began as a SQL Server passthrough returning 1/0, so callers compare it with = 1. PostgreSQL returns 1/0 again and SQL Server resolves to that passthrough, restoring the one spelling that works on both. --- .../labkey/api/data/dialect/BasePostgreSqlDialect.java | 6 +++--- query/src/org/labkey/query/QueryServiceImpl.java | 4 ++-- query/src/org/labkey/query/sql/Method.java | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index ede9d656c29..55496e6830f 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -937,9 +937,9 @@ public boolean supportsIsNumeric() @Override public SQLFragment isNumericExpr(SQLFragment expression) { - // A boolean predicate, not 1/0, to match SQL Server's contract; in SELECT position JDBC's getInt() converts true/false to 1/0. - return new SQLFragment("(CAST((").append(expression) - .append(") AS TEXT) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)$')"); + // 1/0, matching what SQL Server's ISNUMERIC() passthrough returns. + return new SQLFragment("(CASE WHEN CAST((").append(expression) + .append(") AS TEXT) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)$' THEN 1 ELSE 0 END)"); } @Override diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index a24b8b3af4a..110124b5d20 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3786,8 +3786,8 @@ public void testWhereClauseWithUnion() @Test public void testRightAndIsnumeric() throws SQLException { - // Portable LabKey-SQL functions: right() dispatches via the JDBC {fn right} escape; isnumeric() is a - // boolean predicate on both -- (ISNUMERIC(x) = 1) on SQL Server, a regex match on PostgreSQL. + // Portable LabKey-SQL functions: right() dispatches via the JDBC {fn right} escape; isnumeric() reads + // back as 1/0 on both -- ISNUMERIC(x) on SQL Server, a regex-based CASE on PostgreSQL. String sql = "SELECT " + " right('hello', 2) AS r1, " + diff --git a/query/src/org/labkey/query/sql/Method.java b/query/src/org/labkey/query/sql/Method.java index df9a4c2b735..264f0d7bf31 100644 --- a/query/src/org/labkey/query/sql/Method.java +++ b/query/src/org/labkey/query/sql/Method.java @@ -1095,9 +1095,9 @@ public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) } } - // Portable isnumeric() is a boolean predicate on both databases -- (ISNUMERIC(x) = 1) on SQL Server, a regex - // match on PostgreSQL -- so it is valid in CASE WHEN and WHERE, not just a SELECT list. The PostgreSQL regex - // accepts only digits with an optional sign/decimal point, stricter than SQL Server's ISNUMERIC(). + // A regex-based CASE on PostgreSQL; SQL Server resolves isnumeric to the mssqlMethods passthrough and never + // reaches here. Yields 1/0 to match that passthrough, not a boolean as JdbcType.BOOLEAN suggests, so + // isnumeric(x) = 1 works on either database -- don't "fix" the dialects to emit predicates instead. static class IsNumericInfo extends AbstractMethodInfo { IsNumericInfo() @@ -1951,7 +1951,7 @@ private static void addJsonPassthroughMethod(String name, JdbcType type, int min mssqlMethods.put("charindex", new PassthroughMethod("charindex", JdbcType.INTEGER, 2, 3)); mssqlMethods.put("concat_ws", new PassthroughMethod("concat_ws", JdbcType.VARCHAR, 1, Integer.MAX_VALUE)); mssqlMethods.put("difference", new PassthroughMethod("difference", JdbcType.INTEGER, 2, 2)); - // isnumeric is registered in labkeyMethod (portable across PostgreSQL and SQL Server) + mssqlMethods.put("isnumeric", new PassthroughMethod("isnumeric", JdbcType.BOOLEAN, 1, 1)); mssqlMethods.put("len", new PassthroughMethod("len", JdbcType.INTEGER, 1, 1)); mssqlMethods.put("patindex", new PassthroughMethod("patindex", JdbcType.INTEGER, 2, 2)); mssqlMethods.put("quotename", new PassthroughMethod("quotename", JdbcType.VARCHAR, 1, 2)); From 9cf6577ef18866ba1876c50ae054cf33518c7777 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Tue, 15 Sep 2026 14:57:25 -0600 Subject: [PATCH 13/14] Evaluate the weekus() argument once on PostgreSQL The formula needs the argument twice, so it was spelled out twice, duplicating the SQL, its bound parameters and any side effects. A VALUES subquery names it instead. --- .../org/labkey/api/data/dialect/BasePostgreSqlDialect.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index 55496e6830f..ca36bb28973 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -952,9 +952,10 @@ public SQLFragment weekIsoExpr(SQLFragment expression) public SQLFragment weekUsExpr(SQLFragment expression) { // Week 1 is whatever week contains Jan 1, and weeks start Sunday: (day of year + Sunday-based weekday of Jan 1 - 1) / 7, rounded down, plus one. - return new SQLFragment("CAST(FLOOR((EXTRACT(doy FROM (").append(expression) - .append(")) + EXTRACT(dow FROM date_trunc('year', (").append(expression) - .append("))) - 1) / 7) + 1 AS INTEGER)"); + // The VALUES subquery names the argument so it is evaluated once; spelled out twice, a volatile argument could read either side of midnight. + return new SQLFragment("(SELECT CAST(FLOOR((EXTRACT(doy FROM v.d) + EXTRACT(dow FROM date_trunc('year', v.d)) - 1) / 7) + 1 AS INTEGER) FROM (VALUES (CAST(") + .append(expression) + .append(" AS TIMESTAMP))) AS v(d))"); } private class PostgreSqlColumnMetaDataReader extends ColumnMetaDataReader From fdbea6cce303d71f235f2e232b450d8620b65fa5 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Tue, 15 Sep 2026 14:57:33 -0600 Subject: [PATCH 14/14] Emit PIVOT values through QExpr.appendSql() The value was bound as a typed parameter, which needed its JdbcType patched from the pivot column and spent one bind per value per aggregate. Letting the constant emit itself quotes through the dialect instead, with no parameters and no conversion. --- .../src/org/labkey/query/sql/QueryPivot.java | 27 +++---------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/query/src/org/labkey/query/sql/QueryPivot.java b/query/src/org/labkey/query/sql/QueryPivot.java index 293211983eb..bd02f46e4dc 100644 --- a/query/src/org/labkey/query/sql/QueryPivot.java +++ b/query/src/org/labkey/query/sql/QueryPivot.java @@ -15,7 +15,6 @@ */ package org.labkey.query.sql; -import org.apache.commons.beanutils.ConversionException; import org.apache.commons.beanutils.ConvertUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -825,31 +824,13 @@ public SQLFragment getSql() String alias = makePivotColumnAlias(col.getAlias(), pivotValue.getKey()); sql.append(comma).append("MAX(CASE WHEN (").append(_pivotColumn.getValueSql()); if (value instanceof QNull) - { sql.append(" IS NULL"); - } else { - // Bind rather than embed the source text: a value containing ';' or a quote trips SQLFragment's guardrail. - // Postgres needs an explicit parameter type, and wrapConstant() types date/timestamp pivot values as - // QString, so prefer the pivot column's type and fall back to the constant's if it won't convert. - Object bindValue = ((IConstant) value).getValue(); - JdbcType bindType = ((QExpr) value).getJdbcType(); - JdbcType columnType = _pivotColumn.getJdbcType(); - if (null != columnType && JdbcType.OTHER != columnType && columnType != bindType) - { - try - { - bindValue = columnType.convert(bindValue); - bindType = columnType; - } - catch (ConversionException ignored) - { - // keep the constant's own type and value - } - } - sql.append("=?"); - sql.add(bindValue, bindType); + // Let the constant write itself into sql, quoting through the dialect. Never splice in its source text: + // text carrying ';' or an unbalanced quote trips SQLFragment's guardrail. + sql.append("="); + ((QExpr) value).appendSql(sql, _query); } sql.append(") THEN (").append(col.getValueSql()).append(") ELSE NULL END) AS ").appendIdentifier(alias); comma = ",\n";