zabetak commented on code in PR #5494:
URL: https://github.com/apache/hive/pull/5494#discussion_r1803127462
##########
ql/src/test/results/clientpositive/llap/jdbc_filter_expand_row_operator.q.out:
##########
@@ -0,0 +1,41 @@
+PREHOOK: query: CREATE EXTERNAL TABLE book (id int, title varchar(100), author
int)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "jdbc:postgresql://localhost:5432/qtestDB",
+ "hive.sql.dbcp.username" = "qtestuser",
+ "hive.sql.dbcp.password" = "qtestpassword",
+ "hive.sql.table" = "book")
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@book
+POSTHOOK: query: CREATE EXTERNAL TABLE book (id int, title varchar(100),
author int)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "jdbc:postgresql://localhost:5432/qtestDB",
+ "hive.sql.dbcp.username" = "qtestuser",
+ "hive.sql.dbcp.password" = "qtestpassword",
+ "hive.sql.table" = "book")
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@book
+PREHOOK: query: explain cbo
+select * from book
+where id = 0 or (id = 1 and author = 11) or (id = 2 and author = 22)
+PREHOOK: type: QUERY
+PREHOOK: Input: default@book
+#### A masked pattern was here ####
+POSTHOOK: query: explain cbo
+select * from book
+where id = 0 or (id = 1 and author = 11) or (id = 2 and author = 22)
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@book
+#### A masked pattern was here ####
+CBO PLAN:
+HiveJdbcConverter(convention=[JDBC.POSTGRES])
+ JdbcFilter(condition=[AND(OR(=($0, 0), AND(=($0, 1), =($2, 11)), AND(=($0,
2), =($2, 22))), IN($0, 0, 1, 2))])
Review Comment:
The `IN($0, 0, 1, 2)` seems kinda redundant any idea why it's there?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/JDBCExpandExpressionsRule.java:
##########
@@ -178,6 +177,9 @@ protected static class RexTransformIntoOrAndClause extends
RexShuttle {
@Override
public RexNode visitCall(RexCall inputCall) {
RexNode node = super.visitCall(inputCall);
+ if (!RexUtil.isFlat(node)) {
+ node = RexUtil.flatten(rexBuilder, node);
+ }
Review Comment:
If we flatten the call here aren't we risking to create a non-flat
expression during the IN to OR transformation below?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/JDBCExpandExpressionsRule.java:
##########
@@ -159,9 +160,16 @@ public void onMatch(RelOptRuleCall call) {
}
- RexNode analyzeRexNode(RexBuilder rexBuilder, RexNode condition) {
- RexTransformIntoOrAndClause transformIntoInClause = new
RexTransformIntoOrAndClause(rexBuilder);
+ RexNode analyzeRexNode(RelOptCluster cluster, RexNode condition) {
+ RexTransformIntoOrAndClause transformIntoInClause = new
RexTransformIntoOrAndClause(cluster.getRexBuilder());
RexNode newCondition = transformIntoInClause.apply(condition);
+
+ if (!RexUtil.isFlat(newCondition)) {
+ newCondition = new RexSimplify(
Review Comment:
Not sure if there is a bug I haven't read the javadoc/code of the respective
methods. If you think there is a bug feel free to raise a CALCITE ticket.
Apart from that, I am ok with the idea to flatten the expression bottom up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
