zabetak commented on code in PR #4409:
URL: https://github.com/apache/hive/pull/4409#discussion_r1235013284
##########
ql/src/test/results/clientpositive/llap/analyze_npe.q.out:
##########
@@ -159,7 +159,7 @@ STAGE PLANS:
Filter Operator
predicate: c1 is null (type: boolean)
Select Operator
- expressions: null (type: void)
+ expressions: array(null) (type: array<string>)
Review Comment:
I understand the changes in this file but in some databases such nuances
(`null` vs `array(null)`) lead to different results.
I run the following on Postgres and confirmed that plain null and an array
with nulls is not the same thing.
```
create table tbl_arr(ca INT ARRAY);
insert into tbl_arr values ('{1}');
insert into tbl_arr values ('{2,3}');
insert into tbl_arr values (null);
insert into tbl_arr values ('{null}');
select * from tbl_arr;
select * from tbl_arr where ca is null;
select * from tbl_arr where ca[1] is null;
```
Can you please check what Hive does/should do and if the results are
different after your changes. This file has just `EXPLAIN` statements so not
easy to tell.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java:
##########
@@ -756,17 +737,8 @@ public ASTNode visitInputRef(RexInputRef inputRef) {
@Override
public ASTNode visitLiteral(RexLiteral literal) {
- if (RexUtil.isNull(literal) && literal.getType().getSqlTypeName() !=
SqlTypeName.NULL
- && rexBuilder != null) {
- // It is NULL value with different type, we need to introduce a CAST
- // to keep it
- if(nullLiteralMap.containsKey(literal)) {
- return ASTBuilder.literal(literal);
- }
- nullLiteralMap.put(literal, true);
- RexNode r = rexBuilder.makeAbstractCast(literal.getType(), literal);
-
- return r.accept(this);
+ if (RexUtil.isNull(literal) && literal.getType().getSqlTypeName() !=
SqlTypeName.NULL) {
+ return createNullField(literal.getType());
}
return ASTBuilder.literal(literal);
Review Comment:
Maybe in a follow up we could add some unit tests for this visitor and in
particular for this method so that we can test all supported null/not null
types. Not strictly needed as part of this change.
--
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]
