Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/10933 )
Change subject: IMPALA-7288: Fix Codegen Crash in FinalizeModule() ...................................................................... Patch Set 1: > Change LGTM. May be it helps to add some end-to-end test which > exercises the hashT table paths with CHAR type to confirm it's > fixed. Same can be said about the scanner changes. > > In the long run, every codegen changes which include handcrafted IR > should include CHAR type as part of the tests. CHAR type was a part of the manual testing that i did for IMPALA-6177. The thing that I am concerned about is that if we decide to add tests for CHAR for only the changes in this patch, then to get full coverage we would also have to add test cases that exercise all handcrafted IR generation code paths that have failure modes for CHAR -- To view, visit http://gerrit.cloudera.org:8080/10933 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f0b527909a9fb3090996bb7510e4d58350c21b0 Gerrit-Change-Number: 10933 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com> Gerrit-Comment-Date: Thu, 12 Jul 2018 22:58:07 +0000 Gerrit-HasComments: No
