Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20399 )
Change subject: IMPALA-5081: Add codegen_opt_level query option ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/20399/2/be/src/codegen/llvm-codegen-cache.h File be/src/codegen/llvm-codegen-cache.h: http://gerrit.cloudera.org:8080/#/c/20399/2/be/src/codegen/llvm-codegen-cache.h@166 PS2, Line 166: bool Empty() { return engine_pointer == nullptr; } > Not relevant to this change but wouldn't it be better to call Reset() here Thanks Daniel for pointing this out. It makes sense to me. I think it was because previously the struct contained shared_ptr, and somehow needs some hack to initialize it, but now it changes to a pointer. It should be okay if all the codegen caching tests can pass. -- To view, visit http://gerrit.cloudera.org:8080/20399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I371f8758b6552263e91a1fbfd9a6e1c28e1fa2bd Gerrit-Change-Number: 20399 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Fri, 25 Aug 2023 00:09:35 +0000 Gerrit-HasComments: Yes
