feat(read)!: take physical row positions from the parquet reader - #286

Follow-up review of the previous commit, focused on divergence from
official DuckLake.

- The internal physical-position column must never reach a user-visible
  schema. Assert that across all five surfaces that could expose one:
  `SELECT *`, the three CDC table functions, and `information_schema`.
  Official DuckLake has no such column in any output, and ours is an
  implementation detail of a single scan.

- A rowid-projected scan is a positional path, so a predicate on a column
  the file predates now reaches the parquet reader, where the expression
  adapter substitutes the DuckLake `initial_default` before pruning. The
  catalog for this test is written by official DuckDB DuckLake, so it
  doubles as a differential check: our read must agree with what official
  itself returns.

Also harden one assumption the previous commit left implicit. The
deletions feed executes partition 0 of its data-file scan and no others.
That was previously guaranteed by an explicit `UnknownPartitioning(1)`
pin, which the previous commit removed as redundant — the scan has one
file group and `DeletedRowsExec` has no DataFusion children, so nothing
repartitions it. Still true, but no longer stated: were it to stop
holding, rows in the other partitions would be dropped silently, giving a
deletion feed that quietly omits deletions. Refuse instead.

…collision

Review follow-ups. Two correctness fixes and a round of test hardening.

**A recorded float `min_value` is not a lower bound when NaN may be present.**
The max side has been gated on `contains_nan` since #203, on the reasoning that
"NaN can never sit below the min". That holds only for positive NaN. DataFusion
orders floats with `total_cmp`, which is signed: negative NaN sorts below every
value, `-Infinity` included —

    SELECT (-CAST('NaN' AS DOUBLE)) < CAST('-Infinity' AS DOUBLE)  -- true

— and `contains_nan` does not record the sign. So a catalog whose stats carry
finite bounds alongside `contains_nan = true` (official DuckLake's INSERT writes
exactly that) could have a file pruned at plan time while it holds a matching
`-NaN` row: `SELECT ... WHERE x < C` silently short, `DELETE ... WHERE x < C`
reporting zero rows affected with the row still there. Both bounds are now
gated; `float_max_is_bound` becomes `float_bound_is_usable`. Catalogs this crate
writes were never exposed — its writer suppresses both bounds when NaN is
present — so the exposure was foreign catalogs.

**The internal position column could be name-bound to a user column in the CDC
feeds.** `unique_row_pos_name` checked only the file's physical schema, but
`present_catalog_schema` re-presents these scans under the *catalog* names and
`ColumnRenameExec` binds every output field by name. A catalog column called
`__ducklake_row_pos` whose physical name in this file differs — renamed after
the file was written, or absent from it — therefore slipped the check: the
output schema carried the name twice and both fields resolved to the user's
column. Every internal column is Int64, so nothing failed to downcast; the feeds
just reported the wrong rows (the added test shows a deletions feed reporting
none at all). Callers that re-present a scan under other names now pass those
names to `positional_table_schema_reserving`.

Also from review:

- `build_exec_for_partial_file` gains the same "no embedded rowid and no
  `row_id_start`" refusal every other rowid path has, matching the C++
  extension's throw, rather than failing later as a confusing rename error.
- `DeleteFilterExec::try_new` / `RowIdExec::try_new` return an error instead of
  panicking on an out-of-range `pos_index`.
- Comments describing the removed synthesis design are corrected.

Test hardening, after measuring what the fixtures actually produced:

- The `positional_pushdown_tests` fixture wrote a SINGLE row group (the parquet
  default is 1Mi rows), so nothing in that module exercised row-group pruning,
  and byte-range splitting ran at 100% skew — one partition read everything.
  Capped at 5000 rows per group, it now spans 12.
- Added assertions on the reader's own metrics rather than on plan strings: a
  selective predicate prunes 11 of 12 row groups, and `output_rows_skew` proves
  more than one partition reads data. Without these, both optimisations could
  stop working with every test still green.
- Added: a predicate on a renamed column through a positional read (the read
  side of the rename rewriting, previously untested); a predicate on the
  synthetic `rowid` (must not be pushed); the CDC collision above.
- Dropped a statistics-parity test that compared `Absent` to `Absent` and so
  asserted nothing; the parity it claimed is not observable through the public
  API.

Review follow-ups.

`build_exec_for_partial_file` serves two callers: the rowid-projected read and
an ordinary time-travel read, which projects the embedded rowid column away and
needs no lineage at all. The "no embedded rowid and no `row_id_start`" refusal
added in the previous commit fired on both, so a foreign catalog whose merged
partial file records neither would now fail a plain time-travel `SELECT` that
previously read correctly. Gate it on the output actually carrying `rowid`, and
move it below the embedded-snapshot check so a file missing both reports the
column this path actually needs.

Also correct documentation that still asserts the invariant the float min gate
just disproved — `DuckLakeTableColumnStatistics::contains_nan` and
`stats_collect`'s suppression comment both said NaN can only hide above a max.
Left standing, they invite reintroducing the bug they precede.

`present_catalog_schema`'s hazard note is updated to say which half is now
handled (the position column, via the reserved catalog names) and which is not:
`__ducklake_absent_field_<id>`, and the embedded rowid / snapshot-id columns
whose names come from the file footer and so cannot be suffixed. Binding those
by index rather than by name is the durable fix.

Narrow the parallelism claim in the docs and CHANGELOG. Byte-range splitting
substitutes for the hand-rolled row-group partitioning only on read paths, which
run through the physical optimizer. `build_update_scan_with_snapshot` and the
compaction sources are executed directly, so a single-file UPDATE or
rewrite_files now reads on one thread.

Removing `build_row_group_partitions` left three paths reading one file on one
thread: `resolve_positions` (DELETE), `build_update_scan_with_snapshot`
(UPDATE), and the compaction rewrite. The read paths were unaffected — the
physical optimizer's `repartition_file_scans` rule splits their file groups —
but these three build a plan and run it themselves via `physical_plan::collect`
/ `execute`, so no optimizer rule ever sees them. The old hand-rolled
partitioning had baked the split into the scan config, which is why it applied
there at all.

Documenting that as a caveat was the wrong call: the change is meant to make
these paths faster, not slower, and nothing about reader-derived positions
requires giving up parallelism. `DuckLakeTable::split_across_partitions` asks
the source for the byte-range split at construction instead.

Safe for the same reason pruning is: the physical position comes from the
reader, not from arrival order. All three consumers read positions out of the
column — `resolve_positions` collects them into a `HashSet`,
`apply_update_to_batches` extends a cumulative vector from the position column,
and the compaction coalesce already tolerated multi-partition input — so a
byte-range split cannot move a row's position.

Verified the split happens rather than assuming it: with a target of 8
partitions the DELETE source scan reports 8, where before this it reported 1.
Correctness under the split is already covered — every DELETE, UPDATE and
compaction test now runs multi-partition.

Reverts the caveats the previous two commits added to the CHANGELOG and
`docs/physical-row-positions.md`, and corrects `compaction.rs`, which had just
been updated to say partitions are the bin's files one each.

…t is read

Review follow-ups on the two preceding commits.

`split_across_partitions` asked for `target_partitions` outright. DataFusion
splits purely by byte size and has no idea how many row groups a file has, so a
1.5 MiB two-row-group file at 8 partitions produced six that read nothing and
eight that each fetched the footer — these scans install no shared metadata
cache. The partitioning this replaced capped at the row-group count for exactly
that reason; `FileReadConfig` carries `row_group_count` again and the split
honours it. That also bounds compaction, where the fan-out is the product of the
bin size and the per-file split.

The split now also honours `optimizer.repartition_file_scans`. Previously a user
turning it off still got split DELETE/UPDATE scans, because
`DataSourceExec::repartitioned` reads only `repartition_file_min_size` — the
flag is checked by the optimizer rule these paths deliberately never reach.

`delete_prunes_on_a_file_that_embeds_row_ids` did not test what it claimed:
reverting the guard to the buggy form leaves it green. `predicate_is_prunable`
is read only inside `resolve_positions`, whose plan `DuckLakeDeleteExec` builds
and runs itself and which never appears in `EXPLAIN ANALYZE`; the metric the
test asserted came from a plain `SELECT` over a different file, since catalog
statistics eliminate the small UPDATE-written one before planning. Pinned with a
unit test on the guard itself — which does fail when reverted — and the DELETE
half now targets a row inside the rewritten file, where a mis-bound pushed
predicate would prune everything and delete nothing.

`rowids_survive_row_level_filter_pushdown` now asserts `pushdown_rows_pruned`,
so it cannot pass on row-group pruning alone. `RowIdExec::try_new` gains a test
for the case its new check exists to catch: a plain Int64 column, the shape of
the embedded rowid the old Int64-only check accepted.

On divergence, since this changes pruning: gating the float min is an
engine-layer difference from official DuckLake, not a format one. Official gates
only the max, which is right for DuckDB — it normalizes NaN, so `-NaN = NaN` and
either sign sorts above every value. Arrow's `total_cmp` is sign-sensitive, so
`-NaN` sorts below `-Infinity` here, and gating both is what keeps pruning
consistent with the filter semantics this engine actually has. The underlying
ordering difference is inherited from arrow, predates this work, and shows up in
query results rather than only in pruning; documented in COMPATIBILITY.md.

Also: the doc block for `row_pos_virtual_field` had drifted onto
`validate_row_pos_field`; the CHANGELOG now records that the position column
must carry the `parquet.virtual.row_number` extension type and that no public
constructor for one remains; and the pruning claim is narrowed to `DELETE`,
since an UPDATE's source scan pushes no predicate.

Closes out the review findings on the preceding commits.

`predicate_is_prunable` refused a file that renamed ANY data column. Absent
columns take a placeholder physical name, so that flag is true for every file
written before an `ADD COLUMN` — permanently, and for predicates that never
touch the new column. A table that has ever gained a column lost `DELETE`
pruning across its whole history. `FileReadConfig` now carries the catalog
indices whose physical name differs in this file, derived positionally from the
read schema rather than from `name_mapping`, and only a predicate that
references one of them is refused. An index outside the physical schema is also
refused now rather than assumed safe.

The three `debug_assert!`s validating the position column discarded the error
that names the offending field and indexed before bounds-checking, so an
out-of-range index panicked with "index out of bounds" instead of the message.
They are `#[cfg(debug_assertions)]` blocks that surface both. (This was reported
fixed one commit earlier; that edit replaced text with itself and changed
nothing.)

Compaction had no coverage under a split scan — the path where the fan-out is
largest, since `merge_adjacent_files` builds one source scan per file in the bin
and each now splits. Every other compaction fixture is far below
`repartition_file_min_size`, so it stays single-partition however the config is
set. Added a merge over two 60k-row files under a splitting config, asserting no
row is lost or duplicated and every merged row keeps a distinct rowid.

Documented that a single-file `rewrite_data_files` no longer preserves source
row order, for the same reason an unsorted merge never did.