Zoltán Borók-Nagy created IMPALA-15373:
------------------------------------------
Summary: 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
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)
