Skip to content

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

Merged
jbonofre merged 1 commit into
apache:mainfrom
hutiefang76:frank/arrow390-schema-sync-20261024
Nov 15, 2026
Merged

jbonofre merged 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
@jbonofre
jbonofre force-pushed the frank/arrow390-schema-sync-20261024 branch from 17c93ce to 3afa607 Compare November 14, 2026 07:55
@jbonofre
jbonofre merged commit f10e219 into apache:main Nov 15, 2026
22 checks passed
jbonofre added a commit to jbonofre/arrow-java that referenced this pull request Nov 15, 2026
… in VectorSchemaRoot

JsonFileWriter and JsonFileReader look each vector up by position, so
columns with equal Fields no longer share one vector. The Field lookup
keeps the first of such columns, as getVector(String) documents, and
syncSchema() keeps the custom metadata of the schema.

Also addresses review comments on apache#1389: the constructor and
syncSchema() share the code that builds the lookup, and the JSON round
trip after a schema sync moved to TestJSONFile.

Closes apache#1473.
jbonofre added a commit that referenced this pull request Nov 15, 2026
…ctorSchemaRoot (#1474)

## What's Changed

Follow-up to #1389, which this PR is based on: until #1389 is merged,
only the last commit is new.

- `JsonFileWriter` and `JsonFileReader` look each vector up by position
instead of through `getVector(Field)`, so columns with equal Fields
(such as duplicate names with the same type) no longer share one vector.
A JSON round trip of `x: [1], x: [2]` read back `x: [null], x: [2]`
before.
- The Field lookup of `VectorSchemaRoot` keeps the first of the columns
with equal Fields, as `getVector(String)` documents, instead of the
last.
- `syncSchema()` keeps the custom metadata of the schema.
- Review comments on #1389: the constructor and `syncSchema()` share the
code that builds the lookup, the comments say the old keys no longer
match because the promoted Field is structurally different, an assertion
that could not fail is dropped, and the JSON round trip after a schema
sync moved to `TestJSONFile`.

`TestJSONFile` covers a JSON round trip of duplicate columns through
both `read()` and `read(VectorSchemaRoot)`, and `TestVectorSchemaRoot`
covers the lookup of duplicate columns and the metadata after
`syncSchema()`.

Closes #1473.
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