Postgres dialect and PIVOT fixes surfaced by the SNPRC migration

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
@@ -1130,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);
     }
@@ -1170,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()
     {

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 " +

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

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;
@@ -538,6 +539,7 @@ public Map<String, RelationColumn> getAllColumns()
             }
 
             // Add the pivoted aggregate columns grouped by pivot value
+            boolean droppedColumn = false;
             if (!aggs.isEmpty())
             {
                 for (String pivotValue : pivotValues.keySet())
@@ -549,10 +551,26 @@ public Map<String, RelationColumn> 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);
+                        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;
     }
@@ -822,9 +840,35 @@ 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 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(bindValue, bindType);
+                }
                 sql.append(") THEN (").append(col.getValueSql()).append(") ELSE NULL END) AS ").appendIdentifier(alias);
                 comma = ",\n";
             }