Skip to content

Commit 9a991e5

Browse files
committed
[SQL] Insert explicit casts for several function arguments
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
1 parent 0f60666 commit 9a991e5

2 files changed

Lines changed: 57 additions & 22 deletions

File tree

sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/frontend/ExpressionCompiler.java

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,8 @@ node, new DBSPTypeBool(CalciteObject.EMPTY, false), DBSPOpcode.EQ,
946946
return result;
947947
}
948948
case CHAR_LENGTH: {
949+
validateArgCount(node, operationName, ops.size(), 1);
950+
this.ensureString(ops, 0);
949951
return compileFunction(call, node, type, ops, 1);
950952
}
951953
case ST_POINT: {
@@ -1018,8 +1020,7 @@ node, new DBSPTypeBool(CalciteObject.EMPTY, false), DBSPOpcode.EQ,
10181020
ops, 2);
10191021
}
10201022
case "log10":
1021-
case "ln":
1022-
{
1023+
case "ln": {
10231024
// Cast to Double
10241025
this.ensureDouble(ops, 0);
10251026
// See: https://github.com/feldera/feldera/issues/1363
@@ -1028,27 +1029,25 @@ node, new DBSPTypeBool(CalciteObject.EMPTY, false), DBSPOpcode.EQ,
10281029
}
10291030
return compilePolymorphicFunction(call, node, type, ops, 1);
10301031
}
1031-
case "log":
1032-
{
1032+
case "log": {
10331033
// Turn the arguments into Double
1034-
for (int i = 0; i < ops.size(); i++) {
1034+
for (int i = 0; i < ops.size(); i++)
10351035
this.ensureDouble(ops, i);
1036-
}
10371036
return compilePolymorphicFunction(call, node, type, ops, 1, 2);
10381037
}
10391038
case "power": {
1040-
// power(a, .5) -> sqrt(a). This is more precise.
1041-
// Calcite does the opposite conversion.
1042-
assert ops.size() == 2: "Expected two arguments for power function";
1043-
1039+
validateArgCount(node, operationName, ops.size(), 2);
10441040
// convert integer to double
10451041
DBSPExpression firstArg = ops.get(0);
1046-
if (firstArg.type.is(DBSPTypeInteger.class)) {
1042+
if (firstArg.type.is(DBSPTypeInteger.class))
10471043
this.ensureDouble(ops, 0);
1048-
}
1044+
if (ops.get(1).type.is(DBSPTypeInteger.class))
1045+
this.ensureInteger(ops, 1, 32);
10491046

10501047
DBSPExpression argument = ops.get(1);
10511048
if (argument.is(DBSPDecimalLiteral.class)) {
1049+
// power(a, .5) -> sqrt(a). This is more precise.
1050+
// Calcite does the opposite conversion.
10521051
DBSPDecimalLiteral dec = argument.to(DBSPDecimalLiteral.class);
10531052
BigDecimal pointFive = new BigDecimal(5).movePointLeft(1);
10541053
if (!dec.isNull() && Objects.requireNonNull(dec.value).equals(pointFive)) {
@@ -1068,8 +1067,7 @@ node, new DBSPTypeBool(CalciteObject.EMPTY, false), DBSPOpcode.EQ,
10681067
}
10691068
}
10701069

1071-
return compilePolymorphicFunction(call, node, type,
1072-
ops, 2);
1070+
return compilePolymorphicFunction(call, node, type, ops, 2);
10731071
}
10741072
case "pi": {
10751073
return compileFunction(call, node, type, ops, 0);
@@ -1136,13 +1134,21 @@ node, new DBSPTypeBool(CalciteObject.EMPTY, false), DBSPOpcode.EQ,
11361134
this.ensureInteger(ops, 3, 32);
11371135
return compileFunction(module_prefix + getCallName(call), node, type, ops, 3, 4);
11381136
}
1139-
case "chr":
1137+
case "chr": {
1138+
validateArgCount(node, opName, ops.size(), 1);
1139+
this.ensureInteger(ops, 0, 32);
1140+
return compileFunction(call, node, type, ops, 1);
1141+
}
11401142
case "ascii":
11411143
case "lower":
11421144
case "upper":
1145+
case "initcap":
1146+
validateArgCount(node, opName, ops.size(), 1);
1147+
this.ensureString(ops, 0);
1148+
// fall through
11431149
case "to_hex":
11441150
case "octet_length":
1145-
case "initcap": {
1151+
{
11461152
return compileFunction(call, node, type, ops, 1);
11471153
}
11481154
case "cardinality": {
@@ -1173,6 +1179,9 @@ else if (arg0Type.is(DBSPTypeMap.class))
11731179
case "format_date":
11741180
return compileFunction(call, node, type, ops, 2);
11751181
case "replace":
1182+
validateArgCount(node, opName, ops.size(), 3);
1183+
for (int i = 0; i < ops.size(); i++)
1184+
this.ensureString(ops, i);
11761185
return compileFunction(call, node, type, ops, 3);
11771186
case "division":
11781187
return makeBinaryExpression(node, type, DBSPOpcode.DIV, ops);
@@ -1193,6 +1202,7 @@ else if (arg0Type.is(DBSPTypeMap.class))
11931202
if (ops.get(0).type.is(DBSPTypeBinary.class)) {
11941203
module_prefix = "binary::";
11951204
} else {
1205+
this.ensureString(ops, 0);
11961206
module_prefix = "string::";
11971207
}
11981208
return compileFunction(module_prefix + opName, node, type, ops, 2, 3);
@@ -1202,13 +1212,16 @@ else if (arg0Type.is(DBSPTypeMap.class))
12021212
case "concat":
12031213
return makeBinaryExpressions(node, type, DBSPOpcode.CONCAT, ops);
12041214
case "concat_ws": {
1215+
this.ensureString(ops, 0);
12051216
DBSPExpression sep = ops.get(0);
12061217
if (ops.size() == 1)
12071218
return sep.cast(type);
12081219
DBSPExpression accumulator = DBSPStringLiteral.none(type.withMayBeNull(true));
1209-
for (int i = 1; i < ops.size(); i++)
1220+
for (int i = 1; i < ops.size(); i++) {
1221+
this.ensureString(ops, i);
12101222
accumulator = compileFunction(
12111223
call, node, type, Linq.list(sep, accumulator, ops.get(i)), 3);
1224+
}
12121225
return accumulator.cast(type);
12131226
}
12141227
case "now":
@@ -1354,16 +1367,17 @@ else if (arg0Type.is(DBSPTypeMap.class))
13541367
}
13551368
}
13561369
case POSITION: {
1370+
validateArgCount(node, operationName, ops.size(), 2);
13571371
String module_prefix;
13581372
if (ops.get(0).type.is(DBSPTypeBinary.class)) {
13591373
module_prefix = "binary::";
13601374
} else {
1375+
this.ensureString(ops, 0);
13611376
module_prefix = "string::";
13621377
}
13631378
return compileFunction(module_prefix + getCallName(call), node, type, ops, 2);
13641379
}
13651380
case ARRAY_TO_STRING: {
1366-
// Calcite does not enforce the type of the arguments, why?
13671381
this.ensureString(ops, 1);
13681382
if (ops.size() > 2)
13691383
this.ensureString(ops, 2);
@@ -1372,6 +1386,10 @@ else if (arg0Type.is(DBSPTypeMap.class))
13721386
case LIKE:
13731387
// ILIKE will also match LIKE in Calcite, it's just a special case for case-insensitive matching
13741388
case SIMILAR: {
1389+
validateArgCount(node, operationName, ops.size(), 2, 3);
1390+
for (int i = 0; i < ops.size(); i++)
1391+
// Calcite does not enforce the type of the arguments, why?
1392+
this.ensureString(ops, i);
13751393
return compileFunction(call, node, type, ops, 2, 3);
13761394
}
13771395
case FLOOR:
@@ -1419,10 +1437,13 @@ else if (arg0Type.is(DBSPTypeMap.class))
14191437
}
14201438
return new DBSPBinaryExpression(node, type, opcode, op0, index);
14211439
}
1440+
case TRIM:
1441+
validateArgCount(node, operationName, ops.size(), 3);
1442+
this.ensureString(ops, 1);
1443+
this.ensureString(ops, 2);
1444+
// fall through
14221445
case TIMESTAMP_DIFF:
1423-
case TRIM: {
14241446
return compileKeywordFunction(call, node, null, type, ops, 0, 3);
1425-
}
14261447
case TUMBLE: {
14271448
if (ops.size() >= 2) {
14281449
DBSPExpression op = ops.get(1);
@@ -1576,10 +1597,8 @@ else if (!elemType.sameType(arg1.type.withMayBeNull(elemType.mayBeNull))) {
15761597
case ARRAY_DISTINCT: {
15771598
DBSPExpression arg0 = ops.get(0);
15781599
String method = getCallName(call);
1579-
15801600
if (arg0.type.mayBeNull)
15811601
method += "N";
1582-
15831602
return new DBSPApplyExpression(node, method, type, arg0);
15841603
}
15851604
case ARRAY_REVERSE: {
@@ -1709,3 +1728,4 @@ public DBSPCompiler compiler() {
17091728
return this.compiler;
17101729
}
17111730
}
1731+

sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/simple/RegressionTests.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,21 @@ public void latenessType() {
4343
FROM T;""", "Cannot apply '-' to arguments of type '<INTEGER> - <INTERVAL SECOND>'");
4444
}
4545

46+
@Test
47+
public void issue3159() {
48+
this.compileRustTestCase("""
49+
CREATE TABLE t2(c0 TINYINT);
50+
CREATE VIEW v2_optimized AS (SELECT POWER(5, t2.c0) FROM t2);
51+
""");
52+
}
53+
54+
@Test
55+
public void issue3158() {
56+
this.compileRustTestCase("""
57+
CREATE TABLE t3(c0 SMALLINT);
58+
CREATE VIEW v0 AS (SELECT CHR(t3.c0) FROM t3);""");
59+
}
60+
4661
@Test
4762
public void issue3109() {
4863
var ccs = this.getCCS("""

0 commit comments

Comments
 (0)