This is an automated email from the ASF dual-hosted git repository. stigahuang pushed a commit to branch branch-3.4.2 in repository https://gitbox.apache.org/repos/asf/impala.git
commit 96ed0cf8ff7dad0ef14fe8229afae7fa5b07c19c Author: Zoltan Borok-Nagy <[email protected]> AuthorDate: Wed Oct 21 13:13:37 2020 +0200 IMPALA-10257: Relax check for page filtering HdfsParquetScanner::CheckPageFiltering() is a bit too strict. It checks that all column readers agree on the top level rows. Column readers have different strategies to read columns. One strategy reads ahead the Parquet def/rep levels, the other strategy reads levels and values simoultaneously, i.e. no readahead of levels. We calculate the ordinal of the top level row based on the repetition level. This means when we readahead the rep level, the top level row might point to the value to be processed next. While top level row in the other strategy always points to the row that has been completely processed last. Because of this in CheckPageFiltering() we can allow a difference of one between the 'current_row_' values of the column readers. I also got rid of the DCHECK in CheckPageFiltering() and replaced it with a more informative error report. Testing: * added a test to nested-types-parquet-page-index.test Change-Id: I01a570c09eeeb9580f4aa4f6f0de2fe6c7aeb806 Reviewed-on: http://gerrit.cloudera.org:8080/16619 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- be/src/exec/parquet/hdfs-parquet-scanner.cc | 28 +++++++++++++--- .../QueryTest/nested-types-parquet-page-index.test | 38 ++++++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/be/src/exec/parquet/hdfs-parquet-scanner.cc b/be/src/exec/parquet/hdfs-parquet-scanner.cc index dbbe6c583..0afae64db 100644 --- a/be/src/exec/parquet/hdfs-parquet-scanner.cc +++ b/be/src/exec/parquet/hdfs-parquet-scanner.cc @@ -1157,10 +1157,30 @@ Status HdfsParquetScanner::CheckPageFiltering() { int64_t current_row = scalar_readers_[0]->LastProcessedRow(); for (int i = 1; i < scalar_readers_.size(); ++i) { - if (current_row != scalar_readers_[i]->LastProcessedRow()) { - DCHECK(false); - return Status(Substitute( - "Top level rows aren't in sync during page filtering in file $0.", filename())); + int64_t scalar_reader_row = scalar_readers_[i]->LastProcessedRow(); + if (current_row != scalar_reader_row) { + // Column readers have two strategy to read a column: + // 1: Initialize the reader with NextLevels(), then in a loop call ReadValue() then + // NextLevels(). NextLevels() increments 'current_row_' if the repetition level + // is zero. Because we invoke NextLevels() last, 'current_row_' might correspond + // to the row we are going to read next. + // 2: Use the ReadValueBatch() to read a batch of values. In that case we + // simultaneously read the levels and values, so there is no readahead. + // 'current_row_' always corresponds to the row that we have completely read. + // Because in case 1 'current_row_' might correspond to the next row, or the row + // currently being read, we might have a difference of one here. + if (abs(current_row - scalar_reader_row ) > 1) { + return Status(Substitute( + "Top level rows aren't in sync during page filtering. " + "Current row of $0: $1. Current row of $2: $3. Encountered it when " + "processing file $4. For a workaround page filtering can be turned off by " + "setting query option 'parquet_read_page_index' to FALSE.", + scalar_readers_[0]->node_.element->name, + current_row, + scalar_readers_[i]->node_.element->name, + scalar_reader_row, + filename())); + } } } return Status::OK(); diff --git a/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-page-index.test b/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-page-index.test index 8a372f782..497b13cbb 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-page-index.test +++ b/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-page-index.test @@ -711,3 +711,41 @@ where l_partkey < 10 ---- TYPES BIGINT ==== +---- QUERY +#IMPALA-10257: test when the 'top level row' ordinal differs in the column readers. +# 'o_orderpriority' and the 'l_*' columns are both nested columns, but they are +# nested at different levels and being read via different strategies. +select l_shipmode, o_orderpriority, count(*) +from tpch_nested_parquet.customer.c_orders o, o.o_lineitems l +where l_receiptdate < '1992-01-10' +group by l_shipmode, o_orderpriority +---- RESULTS +'AIR','1-URGENT',4 +'AIR','2-HIGH',3 +'AIR','3-MEDIUM',4 +'AIR','4-NOT SPECIFIED',2 +'AIR','5-LOW',1 +'FOB','1-URGENT',3 +'FOB','2-HIGH',1 +'FOB','3-MEDIUM',2 +'FOB','4-NOT SPECIFIED',3 +'FOB','5-LOW',2 +'MAIL','2-HIGH',3 +'MAIL','3-MEDIUM',3 +'MAIL','4-NOT SPECIFIED',3 +'MAIL','5-LOW',1 +'RAIL','2-HIGH',1 +'RAIL','3-MEDIUM',2 +'RAIL','4-NOT SPECIFIED',1 +'RAIL','5-LOW',1 +'REG AIR','2-HIGH',4 +'REG AIR','3-MEDIUM',2 +'REG AIR','5-LOW',2 +'SHIP','1-URGENT',4 +'SHIP','2-HIGH',2 +'SHIP','4-NOT SPECIFIED',2 +'TRUCK','3-MEDIUM',2 +'TRUCK','5-LOW',3 +---- TYPES +STRING, STRING, BIGINT +====
