zabetak commented on code in PR #5291:
URL: https://github.com/apache/hive/pull/5291#discussion_r1651017360
##########
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:
Thanks for the clarification.
Can you please check if we support other non-boolean expressions in the
WHERE clause. For instance:
```sql
WHERE t.stringColumn
WHERE t.intColumn
WHERE t.dateColumn
WHERE 'foo' || 'bar'
```
I would like to understand if both conditions of the `if` clause are
necessary. In other words if we are about to introduce a `CAST` to `BOOLEAN` do
we really care if the `filterExpression` is a literal?
##########
ql/src/test/queries/clientpositive/filter_literals.q:
##########
@@ -0,0 +1,25 @@
+
Review Comment:
Can you also check that the semantics of non-boolean expressions in the
`WHERE` clause are the same with and without CBO and add tests if necessary.
This is to ensure that we are not introducing behavior changes by allowing this
kind of queries to use CBO.
For instance are the following queries return the same results with and
without CBO?
```sql
WHERE 'foo';WHERE 'true';WHERE 'false';
WHERE 1;WHERE 0;WHERE -1;
WHERE null;
```
##########
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:
Thanks for the explanation. Let's do the CAST here then.
Indeed skipping the CAST is an optimization and we don't need to do it
now/here. Let's ignore this point for now.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
