Repository navigation
Conversation
Bounds written before an int->long or float->double promotion keep their 4-byte encoding. apache#3293 fixed decoding them, but only covered the metrics evaluators. Cover the inspect metadata tables too, so the crash reported in apache#3744 can't regress. Closes apache#3744
Open
1 of 3 tasks
This branch has not been deployed
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
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.
Closes #3744
Rationale for this change
#3744 reports that after a spec-allowed promotion (
int→long,float→double), every metadatainspection API raises
struct.erroron tables that still have files written before the promotion. Boundsare stored without a type tag, and older files keep the 4-byte encoding while the current schema type is
8 bytes.
This is already fixed on
mainby #3293 (released in 0.12.0):from_bytes(LongType/DoubleType)now fallsback to 4-byte decoding, and every bounds decode in
inspect.pygoes through it. The issue was reportedagainst 0.11.1, which predates that fix.
#3293 only added coverage for the metrics evaluators, though; the
inspectpath was fixed as a side effectwith no test pinning it. This PR adds that coverage so the crash can't silently come back, e.g. if
inspectmoves to a different decode path (#4297).Are these changes tested?
Test-only change.
test_inspect_bounds_after_type_promotionwrites withint/float, promotes tolong/double, and asserts correct decoded bounds fromentries,files,data_files,all_files,all_data_files, and themanifests()partition summaries.Verified that it fails with the original
struct.errorwhen #3293's change toconversions.pyis reverted,and passes on
main. Full unit suite: 4524 → 4525 passed, no new failures;make lintclean.Are there any user-facing changes?
No.