Skip to content

fix(schema): preserve doc-only schema evolution - #1052

Open
gty404 wants to merge 2 commits into
apache:mainfrom
gty404:fix/schema-doc-evolution
Open

gty404 wants to merge 2 commits into
apache:mainfrom
gty404:fix/schema-doc-evolution

Conversation

@gty404

@gty404 gty404 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Changing only a field's documentation through SetCurrentSchema silently reuses the previous schema ID and discards the new doc. Include doc in SchemaField equality so schema deduplication records doc-only changes, including nested fields, while preserving the existing default-value comparisons.

Add regressions for required/optional fields, nested struct/list/map fields, doc replacement/addition/removal, metadata JSON round trips, identical-schema deduplication, and historical schema ID reuse. Update two metadata deserialization expectations to include the doc already present in their JSON fixtures.

Validation:

  • The three new regression tests failed before the fix and passed afterward in the original checkout.
  • Rebuilt and ran schema_test, table_test, and json_serde_test on the PR branch based on current upstream main.
  • Ran clang-format and git diff --check.

Compare field documentation during schema equality so metadata deduplication retains doc-only updates. Cover nested fields, metadata round trips, unchanged docs, and historical schema reuse; align existing deserialization expectations with their fixture docs.

@zhjwpku zhjwpku left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, seems like an oversight of #96

@zhjwpku zhjwpku added the ready to merge This PR has been approved and it is ready to merge. label Oct 22, 2026
bool SchemaField::Equals(const SchemaField& other) const {
return field_id_ == other.field_id_ && name_ == other.name_ && *type_ == *other.type_ &&
optional_ == other.optional_ &&
optional_ == other.optional_ && doc_ == other.doc_ &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: doc_ is a plain std::string, so no-doc and doc set to empty compare equal here, while Java's Objects.equals at Types.java:983 treats null and empty as different schemas.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge This PR has been approved and it is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants