diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index c2a43f9747a..ca36bb28973 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -937,10 +937,27 @@ public boolean supportsIsNumeric() @Override public SQLFragment isNumericExpr(SQLFragment expression) { + // 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 + 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. + // 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 { private final TableInfo _table; 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 c83c5b95972..110124b5d20 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() emits ISNUMERIC(x) on SQL Server and a regex-based CASE 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() 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, " + @@ -3821,5 +3820,56 @@ public void testRightAndIsnumeric() throws SQLException assertEquals("isnumeric(NULL) on " + dialect, 0, results.getInt("n4")); } } + + // 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 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.toString()); + 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()); + 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 f4829cc4a11..264f0d7bf31 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) @@ -1076,9 +1095,9 @@ 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. + // 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() @@ -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() @@ -1904,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)); diff --git a/query/src/org/labkey/query/sql/QueryPivot.java b/query/src/org/labkey/query/sql/QueryPivot.java index 42175beebcc..bd02f46e4dc 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 + if (null != pvt) + _columns.put(pivotName, pvt); } } } @@ -824,7 +826,12 @@ public SQLFragment getSql() if (value instanceof QNull) sql.append(" IS NULL"); else - sql.append("=").append(value.getSourceText()); + { + // 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"; }