fix(scan): evaluate predicates exactly over variant-bearing parquet files

[yuhao-su]

Stacked on top of #222 → #224 → #225 (moved to the top of the stack so the other three can merge independently; content unchanged, only rebased). A variant column's field id lives on its parquet storage group; the metadata/value leaves are id-less. That broke three legs of the scan predicate path, each of which resolved references through leaf ids only. This PR restructures the path into three layers with one field-id resolution shared by every consumer. The latest revision folds in the fixes from a full self-review pass (all mechanisms adversarially verified against sabotaged code).

Bind

Schema::build_accessors skipped variant fields, so binding any predicate on a variant column failed with "Accessor for Field not found". Variant fields now get a position-only accessor at every struct level, like java's Accessors; reading the value as a datum errors.

IS NULL / IS NOT NULL are the only operators that bind on a variant column — literal conversion (Datum::to) and the NaN float check already reject the rest, matching java's UnboundPredicate#bind — pinned by test_bind_predicates_on_variant_column. Null-check constant folding now also requires every ancestor to be required (java's allAncestorFieldsAreRequired): previously a required nested field under an optional struct folded IS NULL to AlwaysFalse and dropped the ancestor-null rows at plan time.

Field id resolution (one walk instead of four maps)

The per-layer maps (build_field_id_map, arrow-schema map, position-fallback map) each answered "where is this field id" on their own, all keyed on leaf ids — a variant resolved nowhere, and on id-less (migrated) files nowhere twice. They are replaced by one per-file FileFieldResolution built by a single walk over the parquet tree, which alone does the leaf numbering (no hand-rolled arrow traversal left to drift from filter_leaves); only the id source differs:

• embedded parquet ids when the file has any (decided once, on the parquet schema at any depth like java ParquetSchemaUtil.hasIds, and shared with the pipeline's projection branching — projection and filtering can no longer pick different id sources for the same file);
• top-level arrow-schema ids otherwise, which name mapping or position fallback assigned (java writes those back into the MessageType; the arrow schema is this port's equivalent).

Duplicate field ids and malformed field-id metadata are rejected instead of silently last-wins/dropped; only predicate-referenced ids are recorded.

Evaluation

• IS NULL / IS NOT NULL evaluate exactly in the arrow row filter: the group's first leaf is projected, and validity is AND-ed down the group's struct path — a nested s.v binds and evaluates through the ancestor chain (a NULL s makes s.v NULL), the columnar equivalent of java's null-layer accessors.
• Loud errors instead of silent wrong answers for the undecidable/mismatched cases: a variant id resolving to a non-group column, resolved storage without a binary metadata child (checked for filter-only references, not just projected ones), and any nested reference unresolvable on an id-less file (this port's name mapping is top-level-only; java's is path-recursive — the pre-PR behavior was a loud bind failure, and silent missing-column constants would return wrong rows for a present column).
• Filter-batch positions are computed per distinct projected root; variant references reject the non-null-check operators in one place (bound_reference).
• Page-index and row-group metrics evaluators treat variant references as might-match, like java ParquetMetricsRowGroupFilter's variant carve-out. The row-group evaluator now rejects un-rewritten NOT like the page-index one already did (java rewrites NOT at the filter boundary): its not() inverted conservative might-match answers into prune-everything.
• The empty-projection pad is removed: an empty filter projection yields batches with the correct row count on parquet 58, and padding an arbitrary leaf failed scans on map-first files ("partial projection of MapArray") and panicked on zero-leaf files.

Tests

• test_variant_null_predicates_evaluate_exactly: [2] / [1] / [] across {embedded ids, name-mapped, position-fallback} × row_selection {off, on}.
• test_nested_variant_null_predicates_evaluate_exactly and test_two_variants_under_one_struct_root: ancestor-null semantics and shared-root batch positioning.
• test_stray_nested_id_disables_name_mapping_consistently, test_partial_ids_trust_embedded_ids: the single hasIds decision (java semantics) keeps projection and filtering consistent.
• test_missing_column_predicate_on_map_first_file: constant predicates over missing columns no longer project anything.
• test_nested_variant_predicate_errors_without_embedded_ids, test_variant_predicate_on_non_variant_storage_errors: the loud-error paths.
• test_bind_null_checks_consider_optional_ancestors, eval_rejects_not_predicate, test_resolve_file_field_ids (incl. duplicate-id rejection), test_bind_predicates_on_variant_column.
• 1674 lib tests + workspace check clean. Headline mechanisms adversarially verified (reverting the hasIds unification or re-adding the pad turns the new tests red in exactly the reported failure shapes). RW-side iceberg e2e was verified against an earlier revision of this stack via a local [patch]; RW does not exercise the changed paths (it neither pushes variant predicates nor enables row selection).

[chenzl25]

Requesting changes for two blocking scan-correctness issues. Both affect public reader configurations and can silently return the wrong rows.

[chenzl25]

This turns v IS NULL (and every Variant predicate below) into TRUE, but I cannot find the promised downstream re-evaluation: get_row_filter installs this converter result as the Arrow row filter, and TableScan::to_arrow does not apply the bound predicate again after the Parquet stream. The new fixture actually demonstrates the bug: id 1 has a non-NULL Variant and id 2 is NULL, yet both mutually exclusive predicates expect [1, 2]. Please evaluate the Variant predicate here or in a guaranteed downstream filter. The expected results should be [2] for IS NULL, [1] for IS NOT NULL, and [] for v IS NOT NULL AND id = 2.

[yuhao-su]

Fixed in f287d36: IS NULL / IS NOT NULL are now evaluated exactly in the row filter — the group-level field id resolves to the group's first leaf, and the projected struct column's validity is the variant's nullability. Test expectations are [2] / [1] / [] as suggested. The remaining operators cannot bind on a variant column (pinned by test_bind_predicates_on_variant_column), and their converter arms now error instead of returning true.

[chenzl25]

Please also exercise this test with .with_row_selection_enabled(true). I reproduced that configuration returning []: because the Variant root id is absent from the leaf-id map, PageIndexEvaluator::not_null takes MissingColBehavior::CantMatch and builds a skip-all selection before the Arrow row filter runs. An unresolvable Variant must be conservative at the page-index layer (select_all), and the regression should cover both row-selection settings.

[yuhao-su]

Fixed in f287d36: the page-index and row-group metrics evaluators now treat variant references as might-match instead of missing columns, and the test runs under both row_selection settings. Verified the row-selection leg fails ([] vs [1]) with the carve-out removed.

[chenzl25]

The previous two blockers are fixed. This revision still has two blocking correctness gaps in supported Variant/schema-evolution paths.

[chenzl25]

This group map is rebuilt from the original Parquet schema, so it cannot see IDs assigned only to the resolved Arrow schema for an id-less file. build_field_id_map_from_arrow_schema records only leaf metadata, while the position-fallback map also skips top-level groups. A name-mapped or position-fallback Variant therefore misses both maps and bound_variant_reference treats the present column as missing (IS NULL becomes all-true and IS NOT NULL all-false). I reproduced this by removing the embedded IDs from the new fixture and supplying a NameMapping: v IS NULL returned [1, 2] instead of [2] with row selection disabled. Please resolve group IDs from the post-mapping/fallback Arrow schema and cover both id-less paths.

[yuhao-su]

Fixed in 6488547 — restructured rather than patched: the four per-layer maps are replaced by one per-file FileFieldResolution, built from the parquet schema when it embeds ids and otherwise from the post-mapping/fallback arrow schema (the Rust equivalent of java writing ApplyNameMapping/addFallbackIds ids back into the MessageType). The matrix test now covers embedded/name-mapped/position-fallback × both row_selection settings; disabling the arrow-schema source reproduces exactly your [1, 2] vs [2].

[chenzl25]

This root-ID check rejects a valid nested Variant. SchemaBuilder now creates accessors for nested Variants, so s.v IS NULL binds, and the group map resolves the Variant ID to its first leaf; however, get_column_root returns the outer struct, whose ID differs from the nested Variant ID. I reproduced the scan failing because the Variant is not a root column in the Parquet schema. Removing only this check would still test the outer Struct validity because the filter batch is top-level. Please either carry the nested Arrow path (including ancestor nullability) into evaluation or reject nested Variant predicates during binding and document that restriction.

[yuhao-su]

Fixed in 6488547 by carrying the nested path into evaluation: the resolver records each group's struct path, the root-column check is gone, and the row filter ANDs validity down that path — so s.v IS NULL is true when s is NULL, matching java's null-layer accessors. New test covers {s NULL, s.v NULL, s.v set} under both row_selection settings; with the descent disabled it fails [1] vs [1, 2]. Nested variants on id-less files stay unresolvable (this port's name mapping is top-level-only, pre-existing) — noted in the PR body.

[yuhao-su]

A note for re-review context: the bugs across the revisions of this PR fall into two classes with different structural answers.

Class 1 — fragmented field-id resolution. Each layer (projection mask, row filter, row-group metrics, page index) answered "where does field id X live in this file" with its own map, all keyed on leaf ids — a variant (id on the storage group) resolved nowhere, and id-less files resolved differently per layer. This PR closes the class structurally: one FileFieldResolution per file, built by a single walk over the parquet tree (the only leaf-numbering authority), with the has-ids decision made once (java ParquetSchemaUtil.hasIds semantics, any depth) and shared with the pipeline's projection branching.

Class 2 — the exactness contract is unwritten. The arrow RowFilter is the last filter: nothing re-evaluates after it, so every pushdown arm is load-bearing for correctness (the earlier always-true guards, the page-index CantMatch, the row-group not() inversion, the missing-column constants are all this class). Java makes pushdown conservative by written contract — ContentScanTask#residual() hands engines the expression they must still apply, and the modern read path has no in-reader row filter at all (row-group pruning only; GenericReader.applyResidual / engines do the exact row-level work). Notably our own consumers already assume opposite contracts: the DataFusion integration declares Inexact and re-applies the filter after scanning, while RisingWave drops pushed predicates and relies on the row filter being exact.

This PR fixes the class-2 instances reachable through its own paths and turns the undecidable cases into loud errors, but the class remains: any future arm or type is one conservative-shaped mistake away from silently wrong rows (e.g. the pre-existing missing-column lt/lt_eq → always-true, #221). Follow-up proposal: evaluate a residual filter on the transformed output batches (logical schema, one exact place) and demote the row filter + stats evaluators to conservative-only optimizations — most of the variant evaluation machinery here then shrinks back to conservative pass-through, and the #221 family becomes a perf concern instead of a correctness one.