[
https://issues.apache.org/jira/browse/IMPALA-15373?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Zoltán Borók-Nagy reassigned IMPALA-15373:
------------------------------------------
Assignee: Zoltán Borók-Nagy
> Wrong FILE__POSITION when the first materialized column is complex and the
> file has multiple row groups
> -------------------------------------------------------------------------------------------------------
>
> Key: IMPALA-15373
> URL: https://issues.apache.org/jira/browse/IMPALA-15373
> Project: IMPALA
> Issue Type: Bug
> Components: Backend
> Reporter: Zoltán Borók-Nagy
> Assignee: Zoltán Borók-Nagy
> Priority: Major
>
> FILE__POSITION is row-group-relative instead of file-relative whenever the
> Parquet column reader that fills the slot is a complex reader (collection,
> struct or VARIANT) and the data file has more than one row group. Rows in the
> second and later row groups get positions that are too small by the row-group
> offset.
> This is a regression introduced by IMPALA-11780.
> h2. Root cause
> IMPALA-11780 made {{current_row_}} row-group-relative and compensated for it
> where the file position is produced:
> {code:java}
> // parquet-column-readers.cc:1250, BaseScalarColumnReader::Reset()
> - current_row_ = row_group_first_row - 1;
> + row_group_first_row_ = row_group_first_row;
> + current_row_ = -1;
> // parquet-column-readers.h:738,
> ParquetColumnReader::ReadFilePositionNonBatched()
> - *file_pos = LastProcessedRow() + 1;
> + *file_pos = row_group_first_row_ + LastProcessedRow() + 1;
> {code}
> {{row_group_first_row_}} (parquet-column-readers.h:235) is a member of the
> {{ParquetColumnReader}} base, but it is assigned in exactly one place:
> {{BaseScalarColumnReader::Reset()}} (parquet-column-readers.cc:1250).
> {{ComplexColumnReader::Reset()}} (parquet-complex-column-reader.h:53-57) does
> not set it, and
> {{HdfsParquetScanner::InitComplexColumns()}} (hdfs-parquet-scanner.cc:1005,
> :3094) takes no
> row_group_first_row argument. A complex reader therefore keeps the default 0
> forever.
> {{ReadFilePositionNonBatched()}} is called on {{this}} from
> {{StructColumnReader::ReadValueBatch()}} and
> {{CollectionColumnReader::ReadValueBatch()}}, so
> for a complex reader the compensation term is always 0. Before IMPALA-11780
> this path was
> correct, because {{LastProcessedRow()}} delegates to {{children_[0]}}, whose
> {{current_row_}}
> was absolute at the time.
> The file-position slot is attached to {{(*column_readers)[0]}} regardless of
> reader kind
> (hdfs-parquet-scanner.cc:3021-3024). Slot order is analysis/registration
> order (FROM-clause
> refs, then the select list left to right, then WHERE), so the first reader is
> a complex one
> for queries such as {{select file__position, v from t}}, {{select v from t
> where id = 5}} or
> {{select ... from t c, c.c_orders}}.
> h2. The repo already contains the wrong values
> {{QueryTest/virtual-column-file-position-parquet.test:264-279}} has goldens
> for
> {{customer_nested_multiblock_multipage}} (300 rows in 3 row groups of 100).
> The only
> top-level reader in those queries is the {{c_orders}} CollectionColumnReader,
> so it owns the file-position slot.
> || query || golden || true file position ||
> | l_shipdate='1998-11-26' | 80 | 280 |
> | l_partkey = 199994 | 51, 82 | 151, 282 |
> The true positions were read back from
> {{testdata/data/customer_nested_multiblock_multipage.parquet}} with pyarrow.
> The goldens are exactly the true values minus the row-group offsets (100,
> 200, 200). They were added by
> c56cd7b214 itself, so the test has been locking in the bug since 4.3.0.
> h2. Impact
> * Wrong FILE__POSITION values returned to the user.
> * Wrong application of Iceberg V2 position deletes / V3 deletion vectors:
> IcebergScanPlanner
> adds FILE__POSITION to the data scan tuple and IcebergDeleteNode probes
> with it. Deletes
> aimed at row group 1 positions are also applied at the same relative
> offsets in later row
> groups, and deletes aimed at later row groups are missed.
> * Worst case, persistent corruption: a DELETE whose predicate only touches a
> complex or
> VARIANT column materializes only that column, so the complex reader owns
> the slot and
> *wrong positions are written into the new position-delete file / DV*.
> Only files with more than one row group are affected. Impala writes one row
> group per file,
> so this needs data written by Spark, Hive, Trino or Flink (Iceberg defaults:
> 512 MB target
> file, 128 MB row group, i.e. up to ~4 row groups per file).
> h2. Suggested fix
> Give complex readers the offset, e.g.:
> {code:java}
> // parquet-complex-column-reader.h
> void Reset(int64_t row_group_first_row) {
> def_level_ = rep_level_ = ParquetLevel::INVALID_LEVEL;
> pos_current_value_ = ParquetLevel::INVALID_POS;
> row_group_first_row_ = row_group_first_row;
> }
> // hdfs-parquet-scanner.{h,cc}: InitComplexColumns(int64_t
> row_group_first_row), passed at :1005
> {code}
> {{complex_readers_}} already holds every complex reader, including nested
> ones and the
> collection built by {{CreateCountingReader()}}, via the
> {{PartitionReaders()}} recursion.
> An alternative is to take the offset from {{children_[0]}} inside
> {{ReadFilePositionNonBatched()}}. Note that {{LastProcessedRow()}} must stay
> row-group-relative
> either way: it feeds {{skip_row_id}} (hdfs-parquet-scanner.cc:2442) and the
> page-index
> arithmetic in {{SkipRowsInternal()}}.
> The fix must also correct the goldens in
> virtual-column-file-position-parquet.test:268 and
> :275-276 to 280 and 151, 282, and state in the commit message that the old
> values were wrong.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
