JingsongLi commented on code in PR #681:
URL: https://github.com/apache/paimon-rust/pull/681#discussion_r3818250283


##########
crates/paimon/src/spec/index_manifest.rs:
##########
@@ -71,7 +71,8 @@ pub const INDEX_MANIFEST_ENTRY_SCHEMA: &str = r#"{
                     {"name": "_INDEX_FIELD_ID", "type": "int"},
                     {"name": "_EXTRA_FIELD_IDS", "type": ["null", {"type": 
"array", "items": "int"}], "default": null},
                     {"name": "_INDEX_META", "type": ["null", "bytes"], 
"default": null},
-                    {"name": "_SOURCE_META", "type": ["null", "bytes"], 
"default": null}
+                    {"name": "_SOURCE_META", "type": ["null", "bytes"], 
"default": null},
+                    {"name": "_BUILD_SCHEMA_ID", "type": ["null", "long"], 
"default": null}

Review Comment:
   After reviewing the mixed-version write path, I prefer option (1) for this 
PR unless coordinated Java support is shipped first. Moving the field to a 
trailing top-level position lets the old Rust decoder skip it, but an old Java 
`IndexManifestFileHandler` still deserializes entries into its six-field model 
and rewrites the whole combined manifest, so it drops either a nested or 
top-level structured field. That loses provenance and leaves new Rust unable to 
use or rebuild the index safely after evolution. A future compatible design 
should either gate writers on a Java version that preserves the field or place 
the schema ID in an opaque payload old writers round-trip unchanged.



-- 
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]

Reply via email to