Refactor the code that infers the schema of a JSON file#9485
Closed
Rafferty97 wants to merge 7 commits intoapache:mainfrom
Closed
Refactor the code that infers the schema of a JSON file#9485Rafferty97 wants to merge 7 commits intoapache:mainfrom
Rafferty97 wants to merge 7 commits intoapache:mainfrom
Conversation
Rafferty97
force-pushed
the
refactor-json-schema-inference
branch
2 times, most recently
from
4ebebb7 to
5844fb3
Compare
* Simplify `InferredType` and use arena allocation for efficiency * Simplify logic to simple recursive inference and unification * Remove scalar-to-array coersion that doesn't exist in the JSON reader itself * Move `ValueIter` into its own submodule and expose `record_count` via a getter method
Rafferty97
force-pushed
the
refactor-json-schema-inference
branch
from
743341f to
0993953
Compare
Rafferty97
force-pushed
the
refactor-json-schema-inference
branch
from
0d58267 to
f52d8cc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
This PR fixes #9484, and also sets the groundwork for implementing #9482.
I have refactored the code that infers the schema of a JSON file, and specifically:
InferredTypeand use arena allocation for efficiencyValueIterinto its own submodule and exposerecord_countvia a getter methodRationale for this change
While working on #9482, I saw a need and opportunity to refactor the schema inference code for JSON schemas. I also discovered the bug detailed in #9484.
These changes not only make the code more readible and predictable by eliminating a lot of special case handling, but make it trivial to create a new inference function for "single field" JSON reading.
What changes are included in this PR?
arrow-json/src/reader/schema.rsmixed_arrays.jsonas it's no longer valid, and replacedmixed_arrays.json.gzwitharrays.json.gzAre these changes tested?
Yes, the changes pass all existing unit tests - except for one intentionally removed due to the change in behaviour related to #9484 (removing scalar-to-array promotion). There may be scope for adding more tests to improve coverage, but I haven't pulled this thread yet.
Are there any user-facing changes?
There are no API changes, except for the addition of the
record_countmethod.However, the error messages returned by
infer_json_schemaand its cousins will significantly change. They have all been condensed into the one "Incompatible type found during schema inference: {self:?} vs {other:?}" message.Finally, some files that used to generate a valid schema will now return errors. However, this is desirable because those files would have failed to be read by the actual JSON reader anyway - due to the lack of support for scalar-to-array promotion in the JSON reader.