zabetak commented on code in PR #5291:
URL: https://github.com/apache/hive/pull/5291#discussion_r1644104871
##########
iceberg/iceberg-handler/src/test/queries/positive/delete_iceberg_mixed.q:
##########
@@ -6,6 +6,8 @@
--! qt:replace:/("removed-files-size":")\d+/$1#FileSize#/
-- create an unpartitioned table with skip delete data set to false
+set hive.cbo.fallback.strategy=NEVER;
Review Comment:
How are the changes related to this test?
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -3199,18 +3199,13 @@ private RelNode genFilterRelNode(ASTNode filterNode,
RelNode srcRel,
}
private RelNode genFilterRelNode(RexNode filterExpression, RelNode srcRel,
- ImmutableMap<String, Integer> outerNameToPosMap, RowResolver outerRR)
throws SemanticException {
+ ImmutableMap<String, Integer> outerNameToPosMap, RowResolver outerRR) {
if (RexUtil.isLiteral(filterExpression, false)
&& filterExpression.getType().getSqlTypeName() !=
SqlTypeName.BOOLEAN) {
- // queries like select * from t1 where 'foo';
- // Calcite's rule PushFilterThroughProject chokes on it. Arguably, we
- // can insert a cast to
- // boolean in such cases, but since Postgres, Oracle and MS SQL server
- // fail on compile time
- // for such queries, its an arcane corner case, not worth of adding
that
- // complexity.
- throw new CalciteSemanticException("Filter expression with non-boolean
return type.",
- UnsupportedFeature.Filter_expression_with_non_boolean_return_type);
+ // Cast filterExpression of queries like select * from t1 where 'foo'
+ RelDataType booleanType =
srcRel.getCluster().getTypeFactory().createSqlType(SqlTypeName.BOOLEAN);
+ filterExpression = srcRel.getCluster().getRexBuilder()
+ .makeCast(booleanType, filterExpression);
Review Comment:
Why are we doing the CAST at this stage? It seems more natural to do it
during the conversion from AST to Rex during the construction of the
`filterExpression`.
Moreover, I think we can skip the CAST altogether. If we have a string
literal then in the context of a boolean expression this can be either true or
false so we can create the respective boolean literal directly.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -3199,18 +3199,13 @@ private RelNode genFilterRelNode(ASTNode filterNode,
RelNode srcRel,
}
private RelNode genFilterRelNode(RexNode filterExpression, RelNode srcRel,
- ImmutableMap<String, Integer> outerNameToPosMap, RowResolver outerRR)
throws SemanticException {
+ ImmutableMap<String, Integer> outerNameToPosMap, RowResolver outerRR) {
if (RexUtil.isLiteral(filterExpression, false)
&& filterExpression.getType().getSqlTypeName() !=
SqlTypeName.BOOLEAN) {
Review Comment:
Do we support in Hive queries combining string literals with boolean
expressions (e.g., `WHERE 'foo' AND 'bar'`)?
If yes then the if statement here is not sufficient to cover those cases.
--
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
