[core] Introduce id-based writtenFieldIds on DataFileMeta#8531
Open
zhuxiangyi wants to merge 1 commit into
Open
[core] Introduce id-based writtenFieldIds on DataFileMeta#8531zhuxiangyi wants to merge 1 commit into
zhuxiangyi wants to merge 1 commit into
Conversation
Records the columns written in a data file by field id in addition to the existing name-based writeCols. Field ids are stable across column renames and can address nested fields uniformly, so this is groundwork for moving the data-evolution read/write path from names to field ids (see apache#8334 discussion). - DataFileMeta: append a nullable _WRITTEN_FIELD_IDS ARRAY<INT> to SCHEMA and add writtenFieldIds() (default null); thread it through PojoDataFileMeta and the forAppend/create factories. - DataFileMetaSerializer: serialize/deserialize the new field, isNullAt-guarded so old manifests read it as null. - Add DataFileMetaWriteColsLegacySerializer freezing the previous 20-field layout; bump DataSplit (8->9) and CommitMessage (11->12) versions to dispatch old streams to it. - Writers dual-write writtenFieldIds (derived from writeCols field ids) alongside writeCols, so old readers keep working via writeCols. - Add DataEvolutionUtils.writtenFieldIds(file, schemaFetcher) resolving a file's written columns to ids (writtenFieldIds if present, else writeCols names -> ids), for consumers to switch to in a follow-up. Behavior is unchanged; adds compatibility tests for round-trip, the frozen legacy layout and new-stream/old-serializer forward reads.
16bbf39 to
dda6d83
Compare
Contributor
|
Modify data file meta is too complex... Let's go back to your original plan, using field.field. |
Contributor
Author
|
Thanks @JingsongLi. Just to confirm the direction so I don't go back and forth:
If that's right, I'll close #8531, reopen #8334 from draft and rebase it onto the latest master. Sound good? |
Contributor
|
@zhuxiangyi Sounds good. The changes to the original PR are gradually being made and broken down into multiple small PRs for rapid progress. |
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.
Purpose
This is the first refactoring PR toward moving the data-evolution read/write path from column names to field ids, as discussed in #8334 (so that top-level fields and nested fields are treated the same, per @JingsongLi's suggestion).
It records the columns written in a data file by field id (
writtenFieldIds) in addition to the existing name-basedwriteCols. Field ids are stable across column renames and can address nested fields uniformly (every nested field has a globally unique id), which later lets the read/write/commit/compact path drop name-based resolution. This PR is groundwork only — behavior is unchanged.Changes
DataFileMeta: append a nullable_WRITTEN_FIELD_IDS ARRAY<INT>toSCHEMA; addwrittenFieldIds()(defaultnull); thread it throughPojoDataFileMetaand theforAppend/createfactories.DataFileMetaSerializer: serialize/deserialize the new field,isNullAt-guarded so old manifests read it asnull.DataFileMetaWriteColsLegacySerializerfreezing the previous 20-field layout, and bumpDataSplit(8→9) andCommitMessageSerializer(11→12) to dispatch old binary streams to it.writtenFieldIds(derived from thewriteColsfield ids) next towriteCols, so old readers keep working viawriteCols.DataEvolutionUtils.writtenFieldIds(file, schemaFetcher): resolve a file's written columns to ids (writtenFieldIdsif present, elsewriteColsnames → ids). Consumers switch to this in the follow-up PR.Compatibility
null; old binaryDataSplit/CommitMessagestreams are dispatched to the frozen legacy serializer.writeCols.writeColswill only be dropped later, once all supported versions understandwrittenFieldIds.Tests
DataFileMetaWrittenFieldIdsCompatibilityTest: round-trip with/without ids, the frozen legacy layout still deserializes, new-stream/old-serializer forward read, and name→id resolution (incl. row-tracking system fields). ExistingDataSplitCompatibleTest(v1–v8 fixtures),DataEvolutionTableTestand serializer tests remain green.Part of the plan in #8334.