soumyakanti3578 commented on code in PR #5291:
URL: https://github.com/apache/hive/pull/5291#discussion_r1645057190
##########
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:
Unfortunately, when we are converting the string ASTNode `foo`, we don't
have the context that it is part of a filter expression, i.e.,
`StrExprProcessor` doesn't know if `foo` is part of `a IN ('foo', 'bar')` or
`WHERE 'foo'`. Maybe we can create a new child class of `TypeCheckCtx` to
handle this, I will have to look into it.
But I think since more complex expressions containing literals are getting
handled correctly (as explained in another comment), and we only need to check
for the `WHERE 'foo'` case here, I guess this is probably a good place for the
check.
Regarding the second part of your comment, that is a good idea but maybe out
of scope of this PR, and it may change a lot of other plans (from CAST(...) to
true/false). But I will look into it if you want me to - either in this PR or
another :)
I have also added `filter_literals.q` and `filter_literals.q.out` containing
some more tests.
--
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
