Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18526 )

Change subject: IMPALA-10851: Codegen for structs
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/18526/9/be/src/codegen/codegen-anyval-read-write-info.h
File be/src/codegen/codegen-anyval-read-write-info.h:

http://gerrit.cloudera.org:8080/#/c/18526/9/be/src/codegen/codegen-anyval-read-write-info.h@91
PS9, Line 91: hers.
> name idea: CodegenNullableValueRead?
I'm not sure... I think 'nullable' is misleading, CodegenAnyVal is also 
nullable, this is no difference.


http://gerrit.cloudera.org:8080/#/c/18526/9/be/src/exec/hash-table.cc
File be/src/exec/hash-table.cc:

http://gerrit.cloudera.org:8080/#/c/18526/9/be/src/exec/hash-table.cc@1011
PS9, Line 1011:       llvm::PHINode* is_null_phi = 
rwi.CodegenIsNullPhiNode("is_null_phi");
              :       has_null = builder.CreateOr(has_null, is_null_phi, 
"has_null");
              :     }
              :   }
> Add IsNullAsNoolPhi?
Done


http://gerrit.cloudera.org:8080/#/c/18526/9/be/src/exprs/hive-udf-call.cc
File be/src/exprs/hive-udf-call.cc:

http://gerrit.cloudera.org:8080/#/c/18526/9/be/src/exprs/hive-udf-call.cc@a295
PS9, Line 295:
             :
> Have an a NullAsI8() functions?
This has been removed, so I'm not sure adding a function would be beneficial.


http://gerrit.cloudera.org:8080/#/c/18526/9/be/src/exprs/scalar-expr-evaluator.h
File be/src/exprs/scalar-expr-evaluator.h:

http://gerrit.cloudera.org:8080/#/c/18526/9/be/src/exprs/scalar-expr-evaluator.h@291
PS9, Line 291:   // Converts and stores 'val' to 'result_' according to its 
type. Intended to be called
             :   // from codegen code.
             :   void* StoreResult(const AnyVal& val, const ColumnType& type);
             :   static FunctionContext* 
GetFunctionContext(ScalarExprEvaluator* e
> Why is it difficult?
The problem is with the vectors in ColumnType, which are not empty here, so 
ColumnType::ToIR runs into a DCHECK.

But this function does not use the vectors, so I changed it to use ColumnType 
directly, and I clear the vectors at the call site.



--
To view, visit http://gerrit.cloudera.org:8080/18526
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5272c3f095fd9f07877104ee03c8e43d0c4ec0b6
Gerrit-Change-Number: 18526
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 31 Aug 2022 11:43:33 +0000
Gerrit-HasComments: Yes

Reply via email to