zabetak commented on code in PR #5494:
URL: https://github.com/apache/hive/pull/5494#discussion_r1801204645
##########
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:
The simplifier is expensive and sometimes unpredictable. If we only need to
ensure that generate expression is flat can't we just do:
```java
return RexUtil.flatten(transformIntoInClause.apply(condition));
```
##########
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) {
Review Comment:
nit: This refactoring (builder->cluster) increases the likelihood of
conflicts and may not be necessary if we don't use the simplifier so consider
reverting the respective changes.
##########
ql/src/test/queries/clientpositive/jdbc_filter_expand_row_operator.q:
##########
@@ -0,0 +1,21 @@
+--! qt:database:postgres:q_test_book_table.sql
+
+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");
+
+explain cbo
+select * from book
+where id = 0 or (id = 1 and author = 11) or (id = 2 and author = 22);
+
+-- non jdbc plan containing ROW, for reference
+CREATE EXTERNAL TABLE book1 (id int, title varchar(100), author int);
+explain cbo
+select * from book1
+where id = 0 or (id = 1 and author = 11) or (id = 2 and author = 22);
Review Comment:
Why do we need this? The .q.out file contains the reference and expected
plan. At first sight this seems like an unrelated new test case.
--
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
