Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19898 )
Change subject: IMPALA-10186: Fix writing empty parquet page ...................................................................... Patch Set 2: (3 comments) I ended up doing a larger refactor to address the TODO around removing the loop. http://gerrit.cloudera.org:8080/#/c/19898/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19898/1//COMMIT_MSG@9 PS1, Line 9: Fixes > nit: Fixes Done http://gerrit.cloudera.org:8080/#/c/19898/2/be/src/exec/parquet/hdfs-parquet-table-writer.cc File be/src/exec/parquet/hdfs-parquet-table-writer.cc: http://gerrit.cloudera.org:8080/#/c/19898/2/be/src/exec/parquet/hdfs-parquet-table-writer.cc@918 PS2, Line 918: bytes_needed = BytesNeededFor(value); This is not currently an issue, and may actually represent a change in behavior. I couldn't generate a test scenario where ProcessValue would fail for the first value on a new page. http://gerrit.cloudera.org:8080/#/c/19898/1/be/src/util/dict-encoding.h File be/src/util/dict-encoding.h: http://gerrit.cloudera.org:8080/#/c/19898/1/be/src/util/dict-encoding.h@206 PS1, Line 206: for > nit: inline keyword is redundant here, see: https://en.cppreference.com/w/c Done -- To view, visit http://gerrit.cloudera.org:8080/19898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90d30d958f07c6289a1beba1b5df1ab3d7213799 Gerrit-Change-Number: 19898 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 17 May 2023 23:20:33 +0000 Gerrit-HasComments: Yes
