Skip to content

GH-390: Refresh vector lookup when synchronizing the schema - #1389

Open
hutiefang76 wants to merge 1 commit into
apache:mainfrom
hutiefang76:frank/arrow390-schema-sync-20261024
Open

hutiefang76 wants to merge 1 commit into
apache:mainfrom
hutiefang76:frank/arrow390-schema-sync-20261024

Conversation

@hutiefang76

@hutiefang76 hutiefang76 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

What's Changed

After a child type is promoted, syncSchema() updates the schema but leaves the vector lookup keyed by the old Field values. Looking up a vector using the new schema returns null, and JsonFileWriter then throws a NullPointerException. Refresh the Field-keyed lookup in the schema-change branch, preserving vector identity and ordering.

Add regressions using two rows of promoted nested-list data: direct lookup verifies the new Field, removal of the stale key, unchanged values and row count, and a second synchronization that makes no change. A real JSON write/read round trip verifies the schema and both nested row values. The unchanged implementation fails the lookup assertion and throws in the writer.

Validation on JDK 21:

  • mvn -B -ntp -Dmaven.gitcommitid.nativegit=true -pl vector -am verify passed, including dependency modules.
  • The vector module ran 1193 cases in each of its two configured allocator executions, with 4 skipped per execution and no failures or errors. All 11 TestVectorSchemaRoot cases passed in each execution without skips.
  • Spotless and Checkstyle passed. The native Git option handles the existing Git plugin's local worktree compatibility; no build configuration was changed.

Closes #390.

Maintainer note: the CI workflows are action_required, and my account cannot add repository labels. Please add bug-fix and approve the runs when appropriate. Remote CI and maintainer review remain pending.

AI-generated PR description. Investigation, implementation and automated review assisted by Codex and DeepSeek.

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

I checked this on top of current main: both new tests fail without the change (the JSON one with the exact NPE from #390) and pass with it.

@jbonofre jbonofre added the bug-fix PRs that fix a big. label Nov 14, 2026
@apache apache deleted a comment from github-actions Bot Nov 14, 2026
@github-actions github-actions Bot added this to the 20.0.0 milestone Nov 14, 2026
Promoted child types change Field equality, leaving the lookup cache keyed
by the old schema. Rebuild the cache when the schema changes so Field-based
lookups and JSON serialization resolve the current vectors.

Generated-by: Codex and DeepSeek
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PRs that fix a big.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JsonFileWriter fails with NPE after VectorSchemaRoot syncSchema

2 participants