[GitHub] [hive] zabetak commented on a diff in pull request #4409: HIVE-27428: CTAS fails with SemanticException when join subquery has complex type column and false filter predicate

["zabetak (via GitHub)" <gi...@apache.org>]

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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org