Skip to content

Commit f39d548

Browse files
ankurjunejaankurjuneja
committed
Merge branch 'release26.7-SNAPSHOT' into 26.7_fb_instrumentScheduleDateTimeLocalFirefox
2 parents bd0efda + 5feb5cd commit f39d548

8 files changed

Lines changed: 719 additions & 400 deletions

File tree

‎api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,12 @@ public boolean supportsNativeGreatestAndLeast()
924924
return true;
925925
}
926926

927+
@Override
928+
public boolean supportsNativeIsDistinctFrom()
929+
{
930+
return true;
931+
}
932+
927933
@Override
928934
public boolean supportsIsNumeric()
929935
{

‎api/src/org/labkey/api/data/dialect/SqlDialect.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,6 +882,17 @@ public SQLFragment isNumericExpr(SQLFragment expression)
882882
throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement");
883883
}
884884

885+
/**
886+
* Does the dialect natively support the standard "IS [NOT] DISTINCT FROM" predicate? PostgreSQL and Snowflake
887+
* do; SQL Server, MySQL, and Oracle do not (SQL Server added GREATEST/LEAST in recent versions but has never
888+
* added this predicate). Dialects that return false here get a portable CASE-based rewrite instead; see
889+
* Method.IsDistinctFromMethodInfo.
890+
*/
891+
public boolean supportsNativeIsDistinctFrom()
892+
{
893+
return false;
894+
}
895+
885896
public void handleCreateDatabaseException(SQLException e) throws ServletException
886897
{
887898
throw(new ServletException("Can't create database", e));

‎query/src/org/labkey/query/QueryTestCase.jsp‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ d,seven,twelve,day,month,date,duration,guid
892892
assertNull(rs.getColumn(1).getFormat());
893893
}
894894
},
895-
new SqlTest("SELECT 1 AS name @hidden'")
895+
new SqlTest("SELECT 1 AS name @hidden")
896896
{
897897
@Override
898898
protected void validateResults(Results rs) throws Exception
@@ -914,18 +914,10 @@ d,seven,twelve,day,month,date,duration,guid
914914
},
915915
916916
// test operators in THEN expression
917-
new SqlTest("SELECT CASE WHEN 1=1 THEN 'a' || 'b' ELSE 'x' || 'y' END")
918-
);
919-
920-
List<SqlTest> postgres = List.of(
921-
// ORDER BY tests
922-
new SqlTest("SELECT R.day, R.month, R.date FROM R ORDER BY R.date", 3, Rsize),
923-
new SqlTest("SELECT R.day, R.month, R.date FROM R UNION SELECT R.day, R.month, R.date FROM R ORDER BY date"),
924-
new SqlTest("SELECT R.guid FROM R WHERE overlaps(CAST('2001-01-01' AS DATE), CAST('2001-01-10' AS DATE), CAST('2001-01-05' AS DATE), CAST('2001-01-15' AS DATE))", 1, Rsize),
925-
926-
// regression test: field reference in sub-select (https://www.labkey.org/home/Developer/issues/issues-details.view?issueId=43580)
927-
new SqlTest("SELECT (SELECT GROUP_CONCAT(b.displayname, ', ') FROM core.UsersAndGroups b WHERE b.email IN (SELECT UNNEST(STRING_TO_ARRAY(a.title, ',')))) AS procedurename, a.parent.rowid FROM core.containers a ", 2, 1),
917+
new SqlTest("SELECT CASE WHEN 1=1 THEN 'a' || 'b' ELSE 'x' || 'y' END"),
928918
919+
// is_distinct_from()/is_not_distinct_from() are portable: native "IS [NOT] DISTINCT FROM" on PostgreSQL and
920+
// Snowflake, a CASE-based rewrite elsewhere (e.g. SQL Server) -- see IsDistinctFromMethodInfo.getSQL()
929921
new MethodSqlTest("SELECT is_distinct_from(NULL,NULL)", JdbcType.BOOLEAN, false),
930922
new MethodSqlTest("SELECT is_not_distinct_from(NULL,NULL)", JdbcType.BOOLEAN, true),
931923
new MethodSqlTest("SELECT is_distinct_from(1,NULL)", JdbcType.BOOLEAN, true),
@@ -936,6 +928,16 @@ d,seven,twelve,day,month,date,duration,guid
936928
new MethodSqlTest("SELECT is_not_distinct_from(1,2)", JdbcType.BOOLEAN, false)
937929
);
938930
931+
List<SqlTest> postgres = List.of(
932+
// ORDER BY tests
933+
new SqlTest("SELECT R.day, R.month, R.date FROM R ORDER BY R.date", 3, Rsize),
934+
new SqlTest("SELECT R.day, R.month, R.date FROM R UNION SELECT R.day, R.month, R.date FROM R ORDER BY date"),
935+
new SqlTest("SELECT R.guid FROM R WHERE overlaps(CAST('2001-01-01' AS DATE), CAST('2001-01-10' AS DATE), CAST('2001-01-05' AS DATE), CAST('2001-01-15' AS DATE))", 1, Rsize),
936+
937+
// regression test: field reference in sub-select (https://www.labkey.org/home/Developer/issues/issues-details.view?issueId=43580)
938+
new SqlTest("SELECT (SELECT GROUP_CONCAT(b.displayname, ', ') FROM core.UsersAndGroups b WHERE b.email IN (SELECT UNNEST(STRING_TO_ARRAY(a.title, ',')))) AS procedurename, a.parent.rowid FROM core.containers a ", 2, 1)
939+
);
940+
939941
List<SqlTest> postgresOnlyFunctions()
940942
{
941943
int majorVersion = ((BasePostgreSqlDialect) CoreSchema.getInstance().getSqlDialect()).getMajorVersion();

‎query/src/org/labkey/query/controllers/prompts/LabKeySql.md‎

Lines changed: 295 additions & 344 deletions
Large diffs are not rendered by default.

‎query/src/org/labkey/query/sql/Method.java‎

Lines changed: 103 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@
2727
import org.labkey.api.data.CompareType;
2828
import org.labkey.api.data.Container;
2929
import org.labkey.api.data.CoreSchema;
30+
import org.labkey.api.data.DbScope;
3031
import org.labkey.api.data.JdbcType;
3132
import org.labkey.api.data.MethodInfo;
3233
import org.labkey.api.data.MutableColumnInfo;
3334
import org.labkey.api.data.SQLFragment;
35+
import org.labkey.api.data.SqlSelector;
3436
import org.labkey.api.data.TableInfo;
3537
import org.labkey.api.data.dialect.SqlDialect;
3638
import org.labkey.api.module.Module;
@@ -167,7 +169,7 @@ public MethodInfo getMethodInfo()
167169
labkeyMethod.put("cos", new JdbcMethod("cos", JdbcType.DOUBLE, 1, 1));
168170
labkeyMethod.put("cot", new JdbcMethod("cot", JdbcType.DOUBLE, 1, 1));
169171
labkeyMethod.put("curdate", new JdbcMethod("curdate", JdbcType.DATE, 0, 0));
170-
labkeyMethod.put("curtime", new JdbcMethod("curtime", JdbcType.DATE, 0, 0));
172+
labkeyMethod.put("curtime", new JdbcMethod("curtime", JdbcType.TIME, 0, 0));
171173
labkeyMethod.put("dayofmonth", new JdbcMethod("dayofmonth", JdbcType.INTEGER, 1, 1));
172174
labkeyMethod.put("dayofweek", new JdbcMethod("dayofweek", JdbcType.INTEGER, 1, 1));
173175
labkeyMethod.put("dayofyear", new JdbcMethod("dayofyear", JdbcType.INTEGER, 1, 1));
@@ -214,6 +216,22 @@ public JdbcType getJdbcType(JdbcType[] args)
214216
};
215217
}
216218
});
219+
labkeyMethod.put("is_distinct_from", new Method(JdbcType.BOOLEAN, 2, 2)
220+
{
221+
@Override
222+
public MethodInfo getMethodInfo()
223+
{
224+
return new IsDistinctFromMethodInfo(IS);
225+
}
226+
});
227+
labkeyMethod.put("is_not_distinct_from", new Method(JdbcType.BOOLEAN, 2, 2)
228+
{
229+
@Override
230+
public MethodInfo getMethodInfo()
231+
{
232+
return new IsDistinctFromMethodInfo(IS_NOT);
233+
}
234+
});
217235
labkeyMethod.put("isequal", new Method("isequal", JdbcType.BOOLEAN, 2, 2)
218236
{
219237
@Override
@@ -1843,24 +1861,6 @@ public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments)
18431861
postgresMethods.put("jsonb_path_query_tz", new PassthroughMethod("jsonb_path_query_tz", JdbcType.VARCHAR, 2, 4));
18441862
postgresMethods.put("jsonb_path_query_array_tz", new PassthroughMethod("jsonb_path_query_array_tz", JdbcType.VARCHAR, 2, 4));
18451863
postgresMethods.put("jsonb_path_query_first_tz", new PassthroughMethod("jsonb_path_query_first_tz", JdbcType.VARCHAR, 2, 4));
1846-
1847-
// "is distinct from" and "is not distinct from" operators in method form
1848-
labkeyMethod.put("is_distinct_from", new Method(JdbcType.BOOLEAN, 2, 2)
1849-
{
1850-
@Override
1851-
public MethodInfo getMethodInfo()
1852-
{
1853-
return new IsDistinctFromMethodInfo(IS);
1854-
}
1855-
});
1856-
labkeyMethod.put("is_not_distinct_from", new Method(JdbcType.BOOLEAN, 2, 2)
1857-
{
1858-
@Override
1859-
public MethodInfo getMethodInfo()
1860-
{
1861-
return new IsDistinctFromMethodInfo(IS_NOT);
1862-
}
1863-
});
18641864
}
18651865

18661866
private static class IsDistinctFromMethodInfo extends AbstractMethodInfo
@@ -1876,13 +1876,37 @@ private static class IsDistinctFromMethodInfo extends AbstractMethodInfo
18761876
@Override
18771877
public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments)
18781878
{
1879+
SQLFragment a = arguments[0];
1880+
SQLFragment b = arguments[1];
18791881
SQLFragment ret = new SQLFragment();
1880-
ret.append(" ((").append(arguments[0]).append(")");
1881-
if (token == IS)
1882-
ret.append(" IS DISTINCT FROM ");
1882+
1883+
if (dialect.supportsNativeIsDistinctFrom())
1884+
{
1885+
ret.append(" ((").append(a).append(")");
1886+
if (token == IS)
1887+
ret.append(" IS DISTINCT FROM ");
1888+
else
1889+
ret.append(" IS NOT DISTINCT FROM ");
1890+
ret.append("(").append(b).append(")) ");
1891+
}
18831892
else
1884-
ret.append(" IS NOT DISTINCT FROM ");
1885-
ret.append("(").append(arguments[1]).append(")) ");
1893+
{
1894+
// "IS [NOT] DISTINCT FROM" isn't standard/portable SQL -- it's native only on PostgreSQL-family and
1895+
// Snowflake dialects (and recent SQL Server). Elsewhere rewrite as a CASE expression that always
1896+
// evaluates to a real TRUE/FALSE -- never NULL, even when exactly one side is null -- so it behaves
1897+
// the same as the native predicate would.
1898+
1899+
// This is more complicated than the obvious (a=b or a is null and b is null).
1900+
// That expression can return NULL, we need to return only TRUE/FALSE.
1901+
if (token == IS)
1902+
ret.append(" NOT ");
1903+
ret.append("(");
1904+
ret.append("((").append(a).append(") IS NOT NULL AND (").append(b).append(") IS NOT NULL AND (").append(a).append(")=(").append(b).append("))");
1905+
ret.append(" OR ");
1906+
ret.append("((").append(a).append(") IS NULL AND (").append(b).append(") IS NULL)");
1907+
ret.append(")");
1908+
}
1909+
18861910
return ret;
18871911
}
18881912
}
@@ -2013,5 +2037,60 @@ public void testSimpleString()
20132037
assertNotSimpleString(new SQLFragment("SELECT 'test'"));
20142038
assertNotSimpleString(new SQLFragment("'test''string'"));
20152039
}
2040+
2041+
// Exercises both the native and portable-fallback branches of IsDistinctFromMethodInfo.getSQL() against every
2042+
// dialect that's actually connected in this environment, not just whichever dialect the current CI leg happens
2043+
// to be running against. A FROM-less SELECT works everywhere except Oracle, which requires FROM DUAL.
2044+
//
2045+
// Covers both usage contexts: as a selected boolean value (where three-valued logic would otherwise leak NULL
2046+
// for the one-null-argument case) and as a WHERE clause filter (where three-valued logic normally treats NULL
2047+
// as non-matching, so this instead confirms the CASE-based rewrite still evaluates to a real TRUE/FALSE there).
2048+
@Test
2049+
public void testIsDistinctFrom()
2050+
{
2051+
record Case(String a, String b, boolean distinct) {}
2052+
List<Case> cases = List.of(
2053+
new Case("1", "2", true),
2054+
new Case("1", "1", false),
2055+
new Case("NULL", "NULL", false),
2056+
new Case("1", "NULL", true)
2057+
);
2058+
2059+
for (DbScope scope : DbScope.getDbScopesToTest())
2060+
{
2061+
SqlDialect d = scope.getSqlDialect();
2062+
2063+
for (Case c : cases)
2064+
{
2065+
assertIsDistinctFrom(scope, d, IS, c.a(), c.b(), c.distinct());
2066+
assertIsDistinctFrom(scope, d, IS_NOT, c.a(), c.b(), !c.distinct());
2067+
assertIsDistinctFromWhere(scope, d, IS, c.a(), c.b(), c.distinct());
2068+
assertIsDistinctFromWhere(scope, d, IS_NOT, c.a(), c.b(), !c.distinct());
2069+
}
2070+
}
2071+
}
2072+
2073+
private void assertIsDistinctFrom(DbScope scope, SqlDialect d, int token, String a, String b, boolean expected)
2074+
{
2075+
SQLFragment expr = new IsDistinctFromMethodInfo(token).getSQL(d, new SQLFragment[]{new SQLFragment(a), new SQLFragment(b)});
2076+
SQLFragment T = new SQLFragment(d.getBooleanTRUE()), F = new SQLFragment(d.getBooleanFALSE());
2077+
SQLFragment sql = new SQLFragment("SELECT ").append("CASE WHEN ").append(expr).append(" THEN ").append(T).append(" ELSE ").append(F).append(" END");
2078+
if (d.isOracle())
2079+
sql.append(" FROM DUAL");
2080+
Boolean result = new SqlSelector(scope, sql).getObject(Boolean.class);
2081+
assertEquals(d.getClass().getSimpleName() + ": " + sql.toDebugString(), expected, result);
2082+
}
2083+
2084+
private void assertIsDistinctFromWhere(DbScope scope, SqlDialect d, int token, String a, String b, boolean expected)
2085+
{
2086+
SQLFragment expr = new IsDistinctFromMethodInfo(token).getSQL(d, new SQLFragment[]{new SQLFragment(a), new SQLFragment(b)});
2087+
SQLFragment sql = new SQLFragment("SELECT 1");
2088+
if (d.isOracle())
2089+
sql.append(" FROM DUAL");
2090+
sql.append(" WHERE ").append(expr);
2091+
2092+
boolean matched = new SqlSelector(scope, sql).exists();
2093+
assertEquals(d.getClass().getSimpleName() + " (WHERE): " + sql.toDebugString(), expected, matched);
2094+
}
20162095
}
20172096
}

‎query/src/org/labkey/query/sql/QMethodCall.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,9 @@ public QueryParseException fieldCheck(QNode parent, SqlDialect d)
139139
{
140140
if (getMethod(d) == null)
141141
{
142-
return new QueryParseException("Unknown method " + getField().getName(), null, getLine(), getColumn());
142+
String name = getField().getName();
143+
String hint = SqlParser.forUnknownMethod(name, d);
144+
return new QueryParseException("Unknown method " + name + (null == hint ? "" : ". " + hint), null, getLine(), getColumn());
143145
}
144146
return null;
145147
}

‎query/src/org/labkey/query/sql/SqlBase.g‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@ tokens
5757
@lexer::header
5858
{
5959
package org.labkey.query.sql.antlr;
60-
61-
import org.apache.logging.log4j.Logger;
62-
import org.apache.logging.log4j.LogManager;
6360
}
6461

6562

@@ -134,17 +131,14 @@ tokens
134131

135132
@lexer::members
136133
{
137-
Logger _log = LogManager.getLogger(org.labkey.query.sql.SqlParser.class);
138-
139134
protected void setPossibleID(boolean possibleID)
140135
{
141136
}
142137

143-
@Override
144-
public void emitErrorMessage(String msg)
145-
{
146-
_log.debug(msg);
147-
}
138+
// NOTE: lexer errors are reported via reportError(), which SqlParser._SqlLexer overrides to collect
139+
// them as parse errors. Always use _SqlLexer rather than instantiating SqlBaseLexer directly -- the
140+
// default ANTLR emitErrorMessage() prints unmatchable input to System.err and then drops it, letting
141+
// the remaining characters re-lex into a different, valid-looking query.
148142
}
149143

150144

@@ -180,6 +174,9 @@ REGR_SXY : 'regr_sxy';
180174
REGR_SYY : 'regr_syy';
181175
COUNT : 'count';
182176
CROSS : 'cross';
177+
CURRENT_DATE : 'current_date';
178+
CURRENT_TIME : 'current_time';
179+
CURRENT_TIMESTAMP : 'current_timestamp';
183180
DELETE : 'delete';
184181
DISTINCT : 'distinct';
185182
DOT : '.';
@@ -761,6 +758,10 @@ starAtom
761758
primaryExpression
762759
: ARRAY exprList ']' -> ^(METHOD_CALL IDENT["ARRAY_CONSTRUCT"] exprList)
763760
| TEXTARRAY exprList ']' -> ^(METHOD_CALL IDENT["TEXTARRAY_CONSTRUCT"] exprList)
761+
// SQL-standard niladic datetime keywords -- no parens allowed; sugar for curdate()/curtime()/now()
762+
| CURRENT_DATE -> ^(METHOD_CALL IDENT["CURDATE"] ^(EXPR_LIST))
763+
| CURRENT_TIME -> ^(METHOD_CALL IDENT["CURTIME"] ^(EXPR_LIST))
764+
| CURRENT_TIMESTAMP -> ^(METHOD_CALL IDENT["NOW"] ^(EXPR_LIST))
764765
| id=identPrimary
765766
| constant
766767
| OPEN! ( expression | subQuery) CLOSE!

0 commit comments

Comments
 (0)