Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20318 )
Change subject: IMPALA-12306: (Part 2) Make codegen cache tests with symbol emitter more robust ...................................................................... Patch Set 4: Code-Review+1 (2 comments) It looks a great idea to me http://gerrit.cloudera.org:8080/#/c/20318/4/be/src/codegen/codegen-symbol-emitter.cc File be/src/codegen/codegen-symbol-emitter.cc: http://gerrit.cloudera.org:8080/#/c/20318/4/be/src/codegen/codegen-symbol-emitter.cc@58 PS4, Line 58: if (non_freed_objects_ > 0) { I am not very sure if we also need to consider the abnormal case non_freed_objects_ less than 0 in release version, because DCHECK only handles debug version. What do you think? http://gerrit.cloudera.org:8080/#/c/20318/4/be/src/codegen/codegen-symbol-emitter.cc@64 PS4, Line 64: nit. seems no need for the new line -- To view, visit http://gerrit.cloudera.org:8080/20318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I61b9b0de9c896f3de7eb1be7de33d822b1ab70d0 Gerrit-Change-Number: 20318 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Yida Wu <wydbaggio...@gmail.com> Gerrit-Comment-Date: Mon, 13 Nov 2023 07:14:11 +0000 Gerrit-HasComments: Yes
