Gergely Fürnstáhl has posted comments on this change. ( http://gerrit.cloudera.org:8080/19309 )
Change subject: IMPALA-11717: Use rapidjson for printing collections ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/19309/2/be/src/service/hs2-util.cc File be/src/service/hs2-util.cc: http://gerrit.cloudera.org:8080/#/c/19309/2/be/src/service/hs2-util.cc@409 PS2, Line 409: if (is_map) { 4 if(is_map) does not feel right here. At the very least, is_map should be const, but I think the best would be to separate the array/map writing logic into different functions. http://gerrit.cloudera.org:8080/#/c/19309/2/be/src/service/hs2-util.cc@467 PS2, Line 467: element_type.type == TYPE_MAP No need to convert this to bool, using the enum is better for the reader imo, but the best would be still to separate the array and the map logic. -- To view, visit http://gerrit.cloudera.org:8080/19309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I08a2d596a498fbbaf1419b18284846b992f49165 Gerrit-Change-Number: 19309 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Tue, 06 Dec 2022 13:46:50 +0000 Gerrit-HasComments: Yes
