Skip to content

[SQL] Heuristic validation for format arguments #5879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mihaibudiu merged 1 commit into from
Mar 20, 2026
Merged

[SQL] Heuristic validation for format arguments #5879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions ...SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/frontend/ExpressionCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.apache.calcite.util.DateString;
import org.apache.calcite.util.TimeString;
import org.apache.calcite.util.TimestampString;
import org.dbsp.sqlCompiler.circuit.operator.DBSPConcreteAsofJoinOperator;
import org.dbsp.sqlCompiler.compiler.DBSPCompiler;
import org.dbsp.sqlCompiler.compiler.ICompilerComponent;
import org.dbsp.sqlCompiler.compiler.errors.BaseCompilerException;
Expand Down Expand Up @@ -930,6 +931,54 @@ private boolean validateCast(DBSPType from, DBSPType to, boolean safe) {
return false;
}

void checkFormatArg(List<DBSPExpression> args, int formatPosition) {
if (formatPosition >= args.size())
return;
DBSPExpression formatArg = args.get(formatPosition);
DBSPType formatArgType = formatArg.getType();
if (!formatArgType.is(DBSPTypeString.class)) {
this.compiler.reportWarning(formatArg.getSourcePosition(),
"Suspicious argument",
"Format argument is expected to be a string, but the type is " + formatArgType.asSqlString());
return;
}
if (formatArg.is(DBSPStringLiteral.class)) {
String str = formatArg.to(DBSPStringLiteral.class).value;
if (str == null) {
this.compiler.reportWarning(formatArg.getSourcePosition(),
"Suspicious argument",
"Format argument is NULL.");
return;
}
if (!str.contains("%")) {
this.compiler.reportWarning(formatArg.getSourcePosition(),
"Suspicious argument",
"Format argument does not look like a format string.");
return;
}
} else if (formatArg.is(DBSPCastExpression.class)) {
// This may be a sign that the format string is
DBSPCastExpression cast = formatArg.to(DBSPCastExpression.class);
if (!cast.source.getType().is(DBSPTypeString.class)) {
this.compiler.reportWarning(formatArg.getSourcePosition(),
"Suspicious argument",
"Format argument does not look like a format string.");
return;
}
}
if (args.size() == 2) {
DBSPExpression otherArg = args.get(1 - formatPosition);
if (otherArg.is(DBSPStringLiteral.class)) {
String strData = otherArg.to(DBSPStringLiteral.class).value;
if (strData != null && strData.contains("%")) {
this.compiler.reportWarning(otherArg.getSourcePosition(),
"Suspicious argument",
"Are the two arguments swapped?");
}
}
}
}

@Override
public DBSPExpression visitCall(RexCall call) {
CalciteObject node = CalciteObject.create(this.context, call);
Expand Down Expand Up @@ -1377,6 +1426,7 @@ else if (arg0Type.is(DBSPTypeMap.class))
return compilePolymorphicFunction(false, call, node, type, ops, 2);
}
case "format_date":
this.checkFormatArg(ops, 0);
return compileFunction(call, node, type, ops, 2);
case "bround": {
validateArgCount(node, opName, ops.size(), 2);
Expand Down Expand Up @@ -1486,6 +1536,7 @@ else if (arg0Type.is(DBSPTypeMap.class))
case "parse_time":
case "parse_timestamp": {
validateArgCount(node, operationName, ops.size(), 2);
this.checkFormatArg(ops, 0);
ensureString(ops, 0);
ensureString(ops, 1);
return compileStrictFunction(call, node, type, ops, 2);
Expand Down
6 changes: 3 additions & 3 deletions ...ler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/inner/Simplify.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,9 @@ public void postorder(DBSPCastExpression expression) {
LocalDate.parse(str.value, dateFormatter); // executed for exception
result = new DBSPDateLiteral(lit.getNode(), type, new DateString(str.value));
} catch (DateTimeParseException ex) {
this.compiler.reportWarning(expression.getSourcePosition(), "Not a DATE",
" String " + Utilities.singleQuote(str.value) +
" cannot be interpreted as a date");
this.compiler.reportWarning(expression.getSourcePosition(), "Suspicious argument",
"String " + Utilities.singleQuote(str.value) +
" cannot be interpreted as a DATE");
}
} else if (type.is(DBSPTypeTime.class)) {
try {
Expand Down
60 changes: 39 additions & 21 deletions ...-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/MetadataTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,21 +196,50 @@ public void windowPositionTest() throws SQLException, IOException {
Assert.assertTrue(found);
}

DBSPCompiler chattyCompiler() {
DBSPCompiler compiler = this.testCompiler();
compiler.options.languageOptions.throwOnError = false;
compiler.options.ioOptions.quiet = false;
return compiler;
}

@Test
public void issue3341() {
String sql = """
CREATE FUNCTION F(x INTEGER NOT NULL) RETURNS INTEGER;
CREATE TABLE T(x INTEGER);
CREATE VIEW V AS SELECT F(x) FROM T;""";
DBSPCompiler compiler = this.testCompiler();
compiler.options.languageOptions.throwOnError = false;
compiler.options.ioOptions.quiet = false;
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation(sql);
compiler.getFinalCircuit(true);
TestUtil.assertMessagesContain(compiler,
"Argument 0 is nullable, while 'f' expects a not nullable value");
}

@Test
public void testFormatWarnings() {
String sql = "CREATE TABLE T(d DATE);\n" +
"CREATE VIEW V AS SELECT FORMAT_DATE(DATE '2020-10-10', '%Y-%m');";
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation(sql);
compiler.getFinalCircuit(true);
TestUtil.assertMessagesContain(compiler,
"warning: Suspicious argument: String '%Y-%m' cannot be interpreted as a DATE");

compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation("CREATE VIEW V AS SELECT PARSE_TIMESTAMP(10, '%s');");
compiler.getFinalCircuit(true);
TestUtil.assertMessagesContain(compiler, """
Format argument does not look like a format string.
1|CREATE VIEW V AS SELECT PARSE_TIMESTAMP(10, '%s');
^^""");

compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation("CREATE VIEW V AS SELECT PARSE_TIMESTAMP('10', '%s');");
compiler.getFinalCircuit(true);
TestUtil.assertMessagesContain(compiler, "Suspicious argument: Format argument does not look like a format string.");
}

@Test
public void propertiesTest() {
String ddl = """
Expand Down Expand Up @@ -381,8 +410,7 @@ CREATE TABLE T (COL1 INT) WITH (
"url": "localhost"
}]'
);""";
DBSPCompiler compiler = this.testCompiler();
compiler.options.ioOptions.quiet = false;
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation(sql);
// Force compilation
compiler.getFinalCircuit(true);
Expand Down Expand Up @@ -480,8 +508,7 @@ CREATE TABLE T (
'connector' = 'localhost'
);
CREATE VIEW V AS SELECT * FROM T;""";
DBSPCompiler compiler = this.testCompiler();
compiler.options.languageOptions.throwOnError = false;
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation(ddl);
TestUtil.assertMessagesContain(compiler, "Duplicate key");
TestUtil.assertMessagesContain(compiler, "Previous declaration");
Expand All @@ -490,18 +517,14 @@ CREATE TABLE T (
@Test
public void materializedProperty() {
String ddl = "CREATE VIEW V WITH ('materialized' = 'true') AS SELECT 5;";
DBSPCompiler compiler = this.testCompiler();
compiler.options.languageOptions.throwOnError = false;
compiler.options.ioOptions.quiet = false;
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation(ddl);
TestUtil.assertMessagesContain(compiler, "please use 'CREATE MATERIALIZED VIEW' instead");
}

@Test
public void unusedInputColumns() {
DBSPCompiler compiler = this.testCompiler();
compiler.options.languageOptions.throwOnError = false;
compiler.options.ioOptions.quiet = false;
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation("""
CREATE TABLE T(used INTEGER, unused INTEGER);
CREATE TABLE T1(used INTEGER, unused INTEGER) with ('materialized' = 'true');
Expand Down Expand Up @@ -568,8 +591,7 @@ public void testValidateSkip() {

@Test
public void issue5436() {
DBSPCompiler compiler = this.testCompiler();
compiler.options.ioOptions.quiet = false;
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation("""
CREATE TABLE T(used INTEGER, unused INTEGER) with ('skip_unused_columns' = 'true');
CREATE TABLE T1(used INTEGER, unused INTEGER) with (
Expand Down Expand Up @@ -641,9 +663,7 @@ public void trimUnusedInputColumns() {

@Test
public void issue3427() {
DBSPCompiler compiler = this.testCompiler();
compiler.options.languageOptions.throwOnError = false;
compiler.options.ioOptions.quiet = false;
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation("""
CREATE TABLE t1(c1 INTEGER);
CREATE VIEW v1 AS SELECT ELEMENT(ARRAY [2, 3]) FROM t1;""");
Expand All @@ -655,9 +675,7 @@ public void issue3427() {

@Test
public void issue3706() {
DBSPCompiler compiler = this.testCompiler();
compiler.options.languageOptions.throwOnError = false;
compiler.options.ioOptions.quiet = false;
DBSPCompiler compiler = this.chattyCompiler();
compiler.submitStatementsForCompilation("""
CREATE TABLE T(x INT, y INT);
CREATE VIEW V as (SELECT * FROM T) UNION ALL (SELECT y, x FROM T);
Expand Down
Loading