Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22662 )
Change subject: IMPALA-3841: Enable late materialization for collections ...................................................................... Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/22662/8/be/src/exec/parquet/parquet-column-readers.cc File be/src/exec/parquet/parquet-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/22662/8/be/src/exec/parquet/parquet-column-readers.cc@1348 PS8, Line 1348: num_rows_skipped_by_late_materialization_counter_ SkipTopLevelRows() is used in other contexts also, not just with late materialization. Since the data pages are not aligned, we can skip rows due to page filtering, e.g.: Table has two cols, col_a and col_b. col_a has 3 pages, each of which has 100 values. col_b has 1 page with 300 values. If the second page of col_a is skipped, then when we read col_b we need to skip the middle 100 values in the page even if late materialization is not used at all. http://gerrit.cloudera.org:8080/#/c/22662/8/be/src/exec/parquet/parquet-column-readers.cc@1348 PS8, Line 1348: (num_rows_to_skip - *remaining) Isn't it just rows_skipped? Above: num_rows_to_skip = num_rows *remaining = num_rows - rows_skipped num_rows_to_skip - *remaining = num_rows - (num_rows - rows_skipped) = rows_skipped This does not apply to L1408 where num_rows is being modified. -- 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: Wed, 07 May 2025 15:23:38 +0000 Gerrit-HasComments: Yes
