Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20482 )
Change subject: IMPALA-12431: Support reading compressed JSON file ...................................................................... Patch Set 4: (4 comments) Thank you for taking the time out of your busy schedule to review this code! http://gerrit.cloudera.org:8080/#/c/20482/4/be/src/exec/json/hdfs-json-scanner.cc File be/src/exec/json/hdfs-json-scanner.cc: http://gerrit.cloudera.org:8080/#/c/20482/4/be/src/exec/json/hdfs-json-scanner.cc@283 PS4, Line 283: Status HdfsJsonScanner::DecompressFileToBuffer(uint8** buffer, int64_t* bytes_read) { > Most of the codes in the method are copied from HdfsTextScanner::FillByteBu Done http://gerrit.cloudera.org:8080/#/c/20482/4/be/src/exec/json/hdfs-json-scanner.cc@329 PS4, Line 329: Status HdfsJsonScanner::DecompressStreamToBuffer(uint8** buffer, int64_t* bytes_read) { > Most of the codes of this method come from HdfsTextScanner::FillByteBufferC Done http://gerrit.cloudera.org:8080/#/c/20482/4/be/src/exec/json/hdfs-json-scanner.cc@333 PS4, Line 333: data_buffer_pool_->FreeAll(); > Is it safe to free all? Shouldn't we attach the memory to the > previous row batch (its tuple_data_pool)? The JsonParser always copies values instead of referencing them, so it doesn't reference any data in the data_buffer_pool_. Therefore, we don't need attach the memory to the previous row batch. In the Close(), there is similar behavior, so I added two comments to explain the reason. http://gerrit.cloudera.org:8080/#/c/20482/4/be/src/exec/json/hdfs-json-scanner.cc@367 PS4, Line 367: uint8_t** decompressed_buffer, int64_t* decompressed_len, bool *eosr) { > This method seems to be exactly the same as HdfsTextScanner::DecompressBuff Done -- To view, visit http://gerrit.cloudera.org:8080/20482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2471855d97d4cdd51363b321055e6b06aa6d81e8 Gerrit-Change-Number: 20482 Gerrit-PatchSet: 4 Gerrit-Owner: Zihao Ye <eyiz...@163.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Zihao Ye <eyiz...@163.com> Gerrit-Comment-Date: Mon, 11 Dec 2023 12:11:20 +0000 Gerrit-HasComments: Yes
