zhuxiangyi commented on PR #8334: URL: https://github.com/apache/paimon/pull/8334#issuecomment-4905693274
Thanks @JingsongLi, that makes sense — moving the whole path to field id (so a nested leaf id is just another field id, and top-level vs nested are handled the same) is cleaner than the dotted-path approach here, and it also removes the name-ambiguity under rename. Happy to do this as a series of smaller PRs. Here's a concrete plan and the compatibility strategy. **Phase 1 — refactor to field id (no new feature, behavior unchanged)** - **PR-1: introduce `writtenFieldIds` on `DataFileMeta` + serialization compat.** Append a nullable `_WRITTEN_FIELD_IDS ARRAY<INT>` to `DataFileMeta.SCHEMA`, thread it through `PojoDataFileMeta`/factories, and update `DataFileMetaSerializer`. The write side populates it (top-level field ids for now) while still dual-writing the existing `writeCols`. Add a helper that resolves a file's written columns to field ids (`writtenFieldIds` if present, else old `writeCols` names → ids). - **PR-2: switch the consumers to field id.** `RowIdColumnConflictChecker`, `DataEvolutionFileStoreScan`, `DataEvolutionCompactCoordinator/Task`, `DataEvolutionRowIdReassigner`, and the read path (`FormatKey` cache key) all resolve columns by id via that helper. Add `RowType/TableSchema.projectByIds(int[])`. Still top-level only; behavior identical. **Phase 2 — the nested feature on top of the id-based path** - **PR-3: nested sub-field data evolution core.** `writtenFieldIds` may now carry nested **leaf** ids — a whole column is recorded by its own field id, a partially-written struct by the leaf ids actually written — and the read assembly composes structs by leaf id. This lets us delete the `projectByPaths`/`leafPaths`/dotted-path layer entirely; nested and top-level become the same code. - **PR-4 / PR-5: engine entries** (Spark `MERGE INTO`, Flink `data_evolution_merge_into`) producing leaf-id sets. **Compatibility strategy** - *Backward* (new reader, old files): `writtenFieldIds` is a nullable appended field, so old manifests read it as null and fall back to `writeCols`; for the versioned `DataSplit`/`CommitMessage` streams I'll bump the version and add a legacy serializer for the current layout (same pattern as `DataFileMeta12LegacySerializer`). - *Forward* (old reader, new files): as long as we keep **dual-writing** `writeCols` (names) next to `writtenFieldIds`, an old reader simply ignores the extra field and keeps working via `writeCols`. We'd only drop `writeCols` later, once all supported versions understand `writtenFieldIds`. (Tables that actually use the nested feature require the new engine anyway.) **Reuse from this PR**: the read-assembly (`DataEvolutionReadPlanner` / struct reassembly), leaf-level conflict check, the Spark/Flink entry logic and all the tests migrate into the phase-2 PRs; the dotted-path layer is dropped. So this is a re-split, not a rewrite. Does this split and the dual-write compatibility approach look right to you? Any adjustments welcome. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
