Xuebin Su has posted comments on this change. ( http://gerrit.cloudera.org:8080/22662 )
Change subject: IMPALA-3841: Enable late materialization for collections ...................................................................... Patch Set 8: (5 comments) Thanks for reviewing! http://gerrit.cloudera.org:8080/#/c/22662/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22662/2//COMMIT_MSG@13 PS2, Line 13: For a collection column, late materialization takes effect only when the : collection > This can add some miner perf regression, right? I mean that in case there a Thanks! Added in the commit message. http://gerrit.cloudera.org:8080/#/c/22662/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22662/7//COMMIT_MSG@26 PS7, Line 26: root cause. > I know you already have some measurements, could you please add a few numbe Thanks! Added in the commit message. http://gerrit.cloudera.org:8080/#/c/22662/7/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/22662/7/be/src/exec/parquet/hdfs-parquet-scanner.cc@2385 PS7, Line 2385: late_materialization_threshold_ > I think that this is not the right place to calculate this, as this will be Thanks! Changed. http://gerrit.cloudera.org:8080/#/c/22662/7/be/src/exec/parquet/parquet-collection-column-reader.cc File be/src/exec/parquet/parquet-collection-column-reader.cc: http://gerrit.cloudera.org:8080/#/c/22662/7/be/src/exec/parquet/parquet-collection-column-reader.cc@181 PS7, Line 181: CollectionColumnReader:: > nit: do we need to qualify this here? Thanks! This is to follow the convention in CollectionColumnReader::ReadValue(). http://gerrit.cloudera.org:8080/#/c/22662/7/be/src/exec/parquet/parquet-column-readers.cc File be/src/exec/parquet/parquet-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/22662/7/be/src/exec/parquet/parquet-column-readers.cc@1347 PS7, Line 1347: // Increase the counter before returning when we successfully skip the rows. > Could you please add a comment why we increase the counter here? Thanks! Added. -- To view, visit http://gerrit.cloudera.org:8080/22662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia21bdfa6811408d66d74367e0a9520e20951105f Gerrit-Change-Number: 22662 Gerrit-PatchSet: 8 Gerrit-Owner: Xuebin Su <x...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Xuebin Su <x...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 06 May 2025 09:02:38 +0000 Gerrit-HasComments: Yes
