[
https://issues.apache.org/jira/browse/IMPALA-15375?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Zoltán Borók-Nagy resolved IMPALA-15375.
----------------------------------------
Fix Version/s: Impala 5.0.0
Resolution: Fixed
> 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
> Priority: Major
> Fix For: Impala 5.0.0
>
>
> 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)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
