feat(core): port Avro schema comparison and Parquet list normalization - #642
Closed
linliu-code wants to merge 7 commits into
Closed
feat(core): port Avro schema comparison and Parquet list normalization#642linliu-code wants to merge 7 commits into
linliu-code wants to merge 7 commits into
Conversation
Adds the context the merge-on-read file group reader needs and the resolver that derives it from a table's configs. Nothing consumes it yet: the reader itself lands in later changes, and the existing read path is untouched. Resolving the context first gives the reader a defined target before any of it is ported, and puts the read-semantics decisions in one reviewable place rather than spread across the port. Two decisions worth review: - `append_only` is rejected rather than mapped. The merge-on-read reader always merges by record key, so no merge mode reproduces "keep every version". The table config derives `append_only` whenever meta fields are disabled or no ordering field is set, which makes it reachable for ordinary tables, so guessing a mode here would silently change which rows a query returns. - `should_merge_use_record_position` stays off. Log blocks already carry record positions but nothing reads them, so merging by key is what a read does today. Deferred until something reads them: the schema handler, the record context, and the bootstrap flags. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`resolve_instant_range` reproduces the bounds `FileGroupReader::create_instant_range_for_log_file_scan` computes, so that a read through either path admits the same log blocks. Nothing enforced that: the two live in different modules and neither calls the other, so editing one would have drifted silently. Compares the `Debug` rendering rather than field by field, since `InstantRange`'s fields are private. That also means a field added to the struct is covered here for free instead of being silently skipped. Verified the assertion can fail, by mutating the resolver twice and confirming each was caught: flipping `end_inclusive`, and dropping the start timestamp. Widens the legacy method to `pub(crate)`. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the entry point callers will use once the merge engine exists. `read` returns `Unsupported` for now. The entry point lands ahead of the engine so the shape callers depend on is settled before anything is built against it, and so the gap is something a test can point at rather than an absence. It shares a name with the file group reader that serves reads today, which it replaces once the engine behind it is written — so it carries the name it will keep, and the eventual swap is a module deletion rather than a rename touching every call site. Until then the two are told apart by module path, and nothing outside the module uses this one. `read` fails rather than returning an empty batch: an empty batch is indistinguishable from a file slice that genuinely has no rows, which would let an unfinished reader look like it worked. Construction stays separate from reading, so callers wiring up a reader do not have to handle a failure until they ask for rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three fixes from self-review: Configs that steer this crate's own behavior (`hoodie.internal.*`, `hoodie.plan.*`) were being swept into the table properties, which are meant to be what the table declares about itself. A reader has no way to tell a swept-in crate config from a real table property, so they now reach it through neither map. The module carried a blanket `allow(dead_code)` because nothing consumes it yet. That also silenced items dead by mistake — it was already hiding that `MergeMode::CommitTimeOrdering` cannot be constructed, since a table with no ordering field derives `append_only`, which the resolver rejects. Each item now carries its own allow, so a newly dead one still warns, and the unreachable variant says why it is unreachable and what makes it reachable. The reader construction test asserted a field the resolver tests already cover and named a guarantee the type system already gives, since `new` cannot fail. It now checks that the reader hands back the parameters it was built with, using non-default values so substituted defaults would fail it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four modules with no dependencies inside the reader subsystem: read statistics, the profiling macro, the iterator mode, and the input split that names what a read covers. Ported as-is from the merge-on-read reader. Nothing consumes them yet, so each file carries a file-scope allow(dead_code); every item in them is live upstream, so a per-item allow would just be noise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The delete context carries what a read needs to recognize a delete: the marker field and value a payload uses to mark one, resolved from table config. The output converter projects a merged batch down to the columns a caller asked for. Both are leaves — no dependencies inside the reader subsystem. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two schema helpers the merge-on-read reader needs. `avro_schema_utils` decides whether two Avro schemas are projection-equivalent, which is how the reader tells a genuine schema change from a reordering or a narrowing that needs no work. `parquet_list_norm` normalizes the two Parquet list encodings — the legacy two-level repeated group and the modern three-level one — so batches read from files written by different engines line up. Brings the two fixtures its tests read; they are force-added past the `**/data` ignore, the same way the existing fixtures under crates/test/data are tracked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 2, 2026
Contributor
Author
|
Closing: this line is superseded. The merge-on-read work now lives on a single line (#663-#673, plus #675), which #660 carries the full squash for review, and is now up to date with the tip. |
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
1 participant
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.
Stacked on #639, #640, #641 — their commits appear here until they merge. Review only the last commit.
Two schema helpers the merge-on-read reader depends on:
schema::avro_schema_utils— decides whether two Avro schemas are projection-equivalent, so the reader can tell a genuine schema change from a reordering or a narrowing that needs no work.schema::parquet_list_norm— normalizes the legacy two-level repeated-group and modern three-level Parquet list encodings, so batches from files written by different engines line up.Also brings the two parquet fixtures
parquet_list_norm's tests read. They are force-added past the**/dataignore, the same way the existing fixtures undercrates/test/dataare tracked.Unlike the
reader_v2modules these arepubunderschema, matching their upstream placement — nothing calls them yet. Full workspace suite green.🤖 Generated with Claude Code