fix: honor nested projections in the mem-wal LSM scanner - #8970
Draft
hamersaw wants to merge 1 commit into
Draft
Conversation
A projection naming a struct leaf (`meta.a`) was widened back to the whole struct. Every projection surface in the mem-wal scanner resolved names through flat, top-level lookups — `field_with_name` / `column_with_name` against the Arrow schema — so a dotted path either missed entirely or matched only its parent. The two halves of the fix: * Schema. `canonical_output_schema` and `validate_projection_names` now resolve through `lance_core::datatypes::Schema`, which narrows a struct to the selected leaf and merges sibling leaves of one parent into a single field (`meta.a` + `meta.c` -> `meta: Struct<a, c>`). `canonical_output_schema` returns `Result` as a consequence. * Data. The eight dataset arms (base table + SSTable across the scan, point-lookup, vector and FTS planners) project by schema rather than by expression. That distinction is load-bearing: `ProjectionPlan` documents that a partial nested projection cannot be expressed through expressions, which is what `Scanner::project` builds. `Scanner::project_with_schema` is added for it. The six memtable execs share `take_projected_columns`, which trims whole stored columns to the projected shape via `RecordBatchExt::project_by_schema` (recursing structs, lists and maps, preserving null buffers). Both arms therefore emit the same narrowed schema before `UnionExec`, and SSTable reads fetch only the selected sub-columns. Also fixes two inconsistencies found along the way: * `MemTableScanner::output_schema` silently dropped a column that `compute_projection_indices` rejected outright. Both now resolve through one `projected_data_fields` and error alike. * `validate_projection_names` used `Schema::project`, which errors on a missing top-level column but yields an empty struct for a missing *child*, letting `meta.nope` through. It now uses `Schema::resolve`, which checks every path segment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YR3wEnDq7cD2a9v2DdTGsZ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A projection naming a struct leaf is widened back to the whole struct.
SELECT meta.aon a mem-wal table returnsmeta: Struct<a, b>instead ofmeta: Struct<a>.Every projection surface in the mem-wal scanner resolved names through flat, top-level lookups —
field_with_name/column_with_nameagainst the Arrow schema — so a dotted path either missed entirely or matched only its parent.validate_projection_namesrejectedmeta.aoutright;canonical_output_schemasilently dropped it.Fix
Schema.
canonical_output_schemaandvalidate_projection_namesresolve throughlance_core::datatypes::Schema, which already narrows a struct to the selected leaf and merges sibling leaves of one parent into a single field (meta.a+meta.c→meta: Struct<a, c>, viado_project'scandidate_field.merge()).canonical_output_schemareturnsResultas a consequence.Data. The eight dataset arms (base table + SSTable across the scan, point-lookup, vector and FTS planners) now project by schema rather than by expression. That distinction is load-bearing —
ProjectionPlan::from_schemadocuments that a partial nested projection "cannot be done easily using expressions", and expressions are whatScanner::projectbuilds.Scanner::project_with_schemais added for it.The six memtable execs share a new
take_projected_columns, which takes whole stored columns and trims them to the projected shape withRecordBatchExt::project_by_schema— already recursive through structs, lists and maps, and null-buffer preserving. Columns whose type already matches pass through untouched, so a projection with no nested paths costs nothing.Both arms therefore emit the same narrowed schema before
UnionExec, and SSTable reads fetch only the selected sub-columns rather than whole structs.Two inconsistencies fixed along the way
MemTableScanner::output_schemasilently dropped a column thatcompute_projection_indicesrejected outright — the two disagreed on the same input. Both now resolve through oneprojected_data_fieldsand error alike.validate_projection_namesusedSchema::project, which errors on a missing top-level column but returns an empty struct for a missing child — someta.nopepassed validation. It now usesSchema::resolve, which checks every path segment.Tests
projection.rs: four new cases — leaf narrowing, sibling merging at the parent's position, whole-struct passthrough, and nested validation acceptingmeta.awhile rejectingmeta.nope.builder.rs:projecting_a_struct_leaf_narrows_the_memtable_output— end-to-end throughMemTableScanner, asserting the sibling does not survive.mem_walsuite: 692 passed, 0 failed.Lint status
cargo fmt --allclean.cargo clippy -p lance --lib --testsclean on every touched file. I have not run the fullcargo clippy --all --tests --benches -- -D warningsthatAGENTS.mdasks for — the machine this was developed on is disk-bound and the full bench build was not viable. Flagging it explicitly rather than implying it passed; CI will cover it.🤖 Generated with Claude Code
https://claude.ai/code/session_01YR3wEnDq7cD2a9v2DdTGsZ