Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24988 )
Change subject: IMPALA-15443: Fix Parquet stats of widened INT32/INT64/FLOAT columns ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/24988/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24988/1//COMMIT_MSG@28 PS1, Line 28: It also fixes the batch decoders, which did not decode the last values : of a batch that started after the first page, e.g. after a NULL page. : Widened sorted columns now use them, and the regular path of : page-level min/max filters had this bug for all types. I don't understand the scope of this second issue: which function has it and which doesn't? Batch decoder means many things in Impala, I would write the exact function names. Also, could it be exploited before this fix for the widening? http://gerrit.cloudera.org:8080/#/c/24988/1/be/src/exec/parquet/parquet-column-stats.cc File be/src/exec/parquet/parquet-column-stats.cc: http://gerrit.cloudera.org:8080/#/c/24988/1/be/src/exec/parquet/parquet-column-stats.cc@337 PS1, Line 337: DecodeBatchOneBoundsCheckFastTrack Do we actually need this fast track path? 2 values per page doesn't look that perf critical to me. My gut feeling is that these functions are over complicated, which contributed to letting the issue in. My preference would be to remove the fast track and the loop unrolling to make this more understandable. http://gerrit.cloudera.org:8080/#/c/24988/1/testdata/workloads/functional-query/queries/QueryTest/overlap_min_max_filters_on_widened_columns.test File testdata/workloads/functional-query/queries/QueryTest/overlap_min_max_filters_on_widened_columns.test: http://gerrit.cloudera.org:8080/#/c/24988/1/testdata/workloads/functional-query/queries/QueryTest/overlap_min_max_filters_on_widened_columns.test@331 PS1, Line 331: set minmax_filter_threshold=0.0; : set minmax_filter_fast_code_path=verification; : SET RUNTIME_FILTER_WAIT_TIME_MS=$RUNTIME_FILTER_WAIT_TIME_MS; I don't understand a the logic behind choosing these values. Shouldnát we disable RUNTIME_FILTER_WAIT_TIME_MS to make runtime filter tests desterministic? set minmax_filter_threshold=0.0 disables the minmax runtime filters - then why do we set other options at all? -- To view, visit http://gerrit.cloudera.org:8080/24988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia39297e906802ed698178943b9c60b79eb905cdb Gerrit-Change-Number: 24988 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Fri, 02 Oct 2026 15:11:23 +0000 Gerrit-HasComments: Yes
