Zoltán Borók-Nagy created IMPALA-15375:
------------------------------------------
Summary: Conjunct on FILE__POSITION is evaluated before the slot
is materialized when late materialization is used
Key: IMPALA-15375
URL: https://issues.apache.org/jira/browse/IMPALA-15375
Project: IMPALA
Issue Type: Bug
Components: Backend
Reporter: Zoltán Borók-Nagy
Assignee: Zoltán Borók-Nagy
Under Parquet late materialization, a scan conjunct on the FILE__POSITION
virtual column can be evaluated against a scratch tuple whose file-position
slot has not been written yet, so the predicate sees 0 (or stale bytes) instead
of the real position. The query then silently returns too few rows.
*Status: derived by tracing the code, not yet reproduced at runtime.* Repro
below.
h2. Root cause
The file-position slot is filled by {{(*column_readers)[0]}}
(hdfs-parquet-scanner.cc:3021-3024),
whichever column that happens to be. {{DivideFilterAndNonFilterColumnReaders()}}
(hdfs-parquet-scanner.cc:274-291) classifies a reader by its own
{{slot_desc()->id()}} only:
{code:java}
auto slot_desc = column_reader->slot_desc();
if (slot_desc != nullptr && std::find(conjunct_slot_ids_.begin(),
conjunct_slot_ids_.end(),
slot_desc->id()) != conjunct_slot_ids_.end()) {
filter_readers->push_back(column_reader);
} else {
non_filter_readers->push_back(column_reader);
}
{code}
A conjunct on the *file-position* slot therefore does not make the reader that
fills it a filter reader. If that reader ends up in {{non_filter_readers_}},
{{AssembleRows()}} runs the filter pass and {{FilterScratchBatch()}} before the
slot is ever written. {{InitTupleBuffer()}}
(hdfs-scanner.h:637) zeroes tuples of at most 64 bytes and clears only the null
bits for larger ones, so the predicate reads 0 or stale bytes.
h2. Repro (scalar, pre-existing)
{{alltypes_tiny_pages}} is not a bulk-loaded table; create it as
{{test_virtual_column_file_position_parquet}} does, with
{{create_table_from_parquet(self.client, db, 'alltypes_tiny_pages')}} from
{{testdata/data/alltypes_tiny_pages.parquet}}.
{code:sql}
set parquet_late_materialization_threshold=-1; -- control: expected 1 row
select string_col, file__position from alltypes_tiny_pages
where id = 6285 and file__position = 7000;
set parquet_late_materialization_threshold=1; -- predicted: 0 rows
select string_col, file__position from alltypes_tiny_pages
where id = 6285 and file__position = 7000;
{code}
Here {{string_col}} is {{column_readers_[0]}} and a non-filter reader, while
{{id}} is the filter
reader. (The id/position pairing is taken from
{{QueryTest/virtual-column-file-position-parquet.test:3-16}}.)
h2. Why this matters now
It becomes a *new* regression for VARIANT as soon as IMPALA-15142 lifts the
late-materialization exclusion for variant readers. This query is correct today
only because the variant forces
{{AssembleRowsWithoutLateMaterialization()}}:
{code:sql}
select v, file__position from functional_parquet.trino_variant
where id = 31 and file__position > 0;
-- today: 1 row (fallback path)
-- after IMPALA-15142 without this fix: predicted 0 rows
{code}
The same applies to {{_row_id}} on Iceberg V3 tables:
{{IcebergVirtualColumnRewriteRule}}
rewrites it to {{COALESCE(_file_row_id, ICEBERG__FIRST__ROW__ID +
FILE__POSITION)}}, and every
VARIANT table is a V3 table.
Iceberg position-delete and deletion-vector plans are *not* affected: they are
LEFT ANTI joins, so their predicates are join conjuncts and they get no runtime
filters.
h2. Suggested fix
Treat the reader that owns the file-position slot as a filter reader when that
slot is
referenced by a conjunct. About five lines in
{{DivideFilterAndNonFilterColumnReaders()}}, using
the existing accessor {{ParquetColumnReader::file_pos_slot_desc()}}
(parquet-column-readers.h:87).
Note the classification runs at hdfs-parquet-scanner.cc:260, after
{{CreateColumnReaders()}} has attached the slot, so the information is
available.
Forcing a reader into {{filter_readers_}} is safe: if it is a collection it
lands at
{{filter_readers_[0]}} and the existing {{max_rep_level() > 0}} guard falls
back to the non-LM
path; a variant as a filter reader already works today. Falling back to
{{AssembleRowsWithoutLateMaterialization()}} whenever a file-position conjunct
exists would be an even simpler, more conservative alternative.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
