Skip to content
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

[CALCITE-6835] Invalid unparse for IS TRUE,IS FALSE,IS NOT TRUE and I… #4214

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[CALCITE-6835] Invalid unparse for IS TRUE,IS FALSE,IS NOT TRUE and I… #4214

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
50 changes: 50 additions & 0 deletions core/src/main/java/org/apache/calcite/sql/dialect/StarRocksSqlDialect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.calcite.rel.type.RelDataTypeSystemImpl;
import org.apache.calcite.sql.SqlAbstractDateTimeLiteral;
import org.apache.calcite.sql.SqlAlienSystemTypeNameSpec;
import org.apache.calcite.sql.SqlBasicCall;
import org.apache.calcite.sql.SqlBasicTypeNameSpec;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlDataTypeSpec;
Expand All @@ -31,9 +32,12 @@
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlWriter;
import org.apache.calcite.sql.fun.SqlFloorFunction;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.type.SqlTypeName;

import com.google.common.collect.ImmutableList;

import org.checkerframework.checker.nullness.qual.Nullable;

import static org.apache.calcite.util.RelToSqlConverterUtil.unparseHiveTrim;
Expand Down Expand Up @@ -128,6 +132,52 @@ public StarRocksSqlDialect(Context context) {
timeUnitNode.getParserPosition());
SqlFloorFunction.unparseDatetimeFunction(writer, newCall, "DATE_TRUNC", false);
break;
case IS_TRUE:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't work if the expression has side effects, e.g., the function is RAND().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I overlooked this non-deterministic expression. If this expression is non-deterministic, I will throw an exception. Or do you have any other more general approaches for handling this situation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only solution I can think of is more complicated: you need to add a new project saving the value of the expression, and then do the expansion for that value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I understand your approach. For this PR, I will throw an exception for non-deterministic expressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we'll consider your suggestion later.

// A IS TRUE -> A IS NOT NULL AND A
SqlCall isNotNullFunc =
new SqlBasicCall(SqlStdOperatorTable.IS_NOT_NULL,
ImmutableList.of(call.operand(0)), SqlParserPos.ZERO);
SqlCall andFunc =
new SqlBasicCall(SqlStdOperatorTable.AND,
ImmutableList.of(isNotNullFunc, call.operand(0)), SqlParserPos.ZERO);
andFunc.unparse(writer, leftPrec, rightPrec);
break;
case IS_NOT_TRUE:
// A IS NOT TRUE -> A IS NULL OR NOT A
SqlCall isNullFunc =
new SqlBasicCall(SqlStdOperatorTable.IS_NULL,
ImmutableList.of(call.operand(0)), SqlParserPos.ZERO);
SqlCall notFunc =
new SqlBasicCall(SqlStdOperatorTable.NOT,
ImmutableList.of(call.operand(0)), SqlParserPos.ZERO);
SqlCall orFunc =
new SqlBasicCall(SqlStdOperatorTable.OR,
ImmutableList.of(isNullFunc, notFunc), SqlParserPos.ZERO);
orFunc.unparse(writer, leftPrec, rightPrec);
break;
case IS_FALSE:
// A IS FALSE -> A IS NOT NULL AND NOT A
SqlCall isNotNullFunc1 =
new SqlBasicCall(SqlStdOperatorTable.IS_NOT_NULL,
ImmutableList.of(call.operand(0)), SqlParserPos.ZERO);
SqlCall notFunc1 =
new SqlBasicCall(SqlStdOperatorTable.NOT,
ImmutableList.of(call.operand(0)), SqlParserPos.ZERO);
SqlCall andFunc1 =
new SqlBasicCall(SqlStdOperatorTable.AND,
ImmutableList.of(isNotNullFunc1, notFunc1), SqlParserPos.ZERO);
andFunc1.unparse(writer, leftPrec, rightPrec);
break;
case IS_NOT_FALSE:
// A IS NOT FALSE -> A IS NULL OR A
SqlCall isNullFunc1 =
new SqlBasicCall(SqlStdOperatorTable.IS_NULL,
ImmutableList.of(call.operand(0)), SqlParserPos.ZERO);
SqlCall orFunc1 =
new SqlBasicCall(SqlStdOperatorTable.OR,
ImmutableList.of(isNullFunc1, call.operand(0)), SqlParserPos.ZERO);
orFunc1.unparse(writer, leftPrec, rightPrec);
break;
default:
super.unparseCall(writer, call, leftPrec, rightPrec);
break;
Expand Down
62 changes: 61 additions & 1 deletion core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ private static String toSql(RelNode root, SqlDialect dialect,
+ "WHERE `product_id` > 0\n"
+ "GROUP BY `product_id`";
final String expectedStarRocks = "SELECT"
+ " SUM(CASE WHEN `net_weight` > 0E0 IS TRUE"
+ " SUM(CASE WHEN `net_weight` > 0E0 IS NOT NULL AND `net_weight` > 0E0"
+ " THEN `shelf_width` ELSE NULL END), SUM(`shelf_width`)\n"
+ "FROM `foodmart`.`product`\n"
+ "WHERE `product_id` > 0\n"
Expand Down Expand Up @@ -8359,6 +8359,66 @@ private void checkLiteral2(String expression, String expected) {
.ok(expected);
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-6835">[CALCITE-6835]
* Invalid unparse for IS TRUE,IS FALSE,IS NOT TRUE and IS NOT FALSE
* in StarRocksDialect</a>. */
@Test void testIsTrue() {
final String sql = "SELECT * FROM \"EMP\" WHERE \"COMM\" > 0 IS TRUE";
final String expected = "SELECT *\n"
+ "FROM \"SCOTT\".\"EMP\"\n"
+ "WHERE CAST(\"COMM\" AS DECIMAL(12, 2)) > 0.00 IS TRUE";
String expectedStarRocks = "SELECT *\n"
+ "FROM `SCOTT`.`EMP`\n"
+ "WHERE (CAST(`COMM` AS DECIMAL(12, 2)) > 0.00 IS NOT NULL AND CAST(`COMM` AS DECIMAL(12, 2)) > 0.00)";
sql(sql)
.schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
.ok(expected)
.withStarRocks().ok(expectedStarRocks);
}

@Test void testIsNotTrue() {
final String sql = "SELECT * FROM \"EMP\" WHERE \"COMM\" > 0 IS NOT TRUE";
final String expected = "SELECT *\n"
+ "FROM \"SCOTT\".\"EMP\"\n"
+ "WHERE CAST(\"COMM\" AS DECIMAL(12, 2)) > 0.00 IS NOT TRUE";
String expectedStarRocks = "SELECT *\n"
+ "FROM `SCOTT`.`EMP`\n"
+ "WHERE (CAST(`COMM` AS DECIMAL(12, 2)) > 0.00 IS NULL OR NOT CAST(`COMM` AS DECIMAL(12, 2)) > 0.00)";
sql(sql)
.schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
.ok(expected)
.withStarRocks().ok(expectedStarRocks);
}

@Test void testIsFalse() {
final String sql = "SELECT * FROM \"EMP\" WHERE \"COMM\" > 0 IS FALSE";
final String expected = "SELECT *\n"
+ "FROM \"SCOTT\".\"EMP\"\n"
+ "WHERE CAST(\"COMM\" AS DECIMAL(12, 2)) > 0.00 IS FALSE";
String expectedStarRocks = "SELECT *\n"
+ "FROM `SCOTT`.`EMP`\n"
+ "WHERE (CAST(`COMM` AS DECIMAL(12, 2)) > 0.00 IS NOT NULL AND NOT CAST(`COMM` AS DECIMAL(12, 2)) > 0.00)";
sql(sql)
.schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
.ok(expected)
.withStarRocks().ok(expectedStarRocks);
}

@Test void testIsNotFalse() {
final String sql = "SELECT * FROM \"EMP\" WHERE \"COMM\" > 0 IS NOT FALSE";
final String expected = "SELECT *\n"
+ "FROM \"SCOTT\".\"EMP\"\n"
+ "WHERE CAST(\"COMM\" AS DECIMAL(12, 2)) > 0.00 IS NOT FALSE";
String expectedStarRocks = "SELECT *\n"
+ "FROM `SCOTT`.`EMP`\n"
+ "WHERE (CAST(`COMM` AS DECIMAL(12, 2)) > 0.00 IS NULL OR CAST(`COMM` AS DECIMAL(12, 2)) > 0.00)";
sql(sql)
.schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
.ok(expected)
.withStarRocks().ok(expectedStarRocks);
}

@Test void testMerge() {
final String sql1 = "merge into \"DEPT\" as \"t\"\n"
+ "using \"DEPT\" as \"s\"\n"
Expand Down