fix(inspector): walk full ArrayNode tree for usedEncodings - #315
Merged
Conversation
InspectorTree's peekFlatRoot only ever resolved a Flat segment's root ArrayNode, so encodings living purely inside another node's tree (e.g. vortex.fsst nested under a vortex.masked root, as CascadingCompressor genuinely produces for high-cardinality Utf8 columns) never showed up in usedEncodings, making `vortex inspect` and anything built on it unreliable for auditing which encoding a file actually uses. collectEncodings now walks the full ArrayNode tree (root + all descendants). Since this newly makes the inspector traverse wire-supplied children it previously never touched, harden the walk to match the untrusted-input contract the reader module's analogous tree walks already follow: - bounds-check the wire-supplied encoding index before resolving it against the footer's array spec table (mirrors PostscriptParser.convertLayout's guard), instead of letting a crafted out-of-range index surface a raw IndexOutOfBoundsException - cap total visited nodes, not just depth: two children vector slots can alias the same absolute table position, so a crafted segment with aliased fan-out could otherwise drive an exponential number of visits while depth stays within the existing MAX_ENCODING_TREE_DEPTH guard Also fixes a stale test comment/assertion in DictValuesPoolCompressionIntegrationTest that claimed the nested FSST values-pool encoding "is not visible in usedEncodings" - it now is, so assert it directly instead of only asserting the vortex.dict wrapper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
InspectorTree'susedEncodings(surfaced by the CLIinspectcommand) only ever inspected a Flat segment's rootArrayNode, so encodings nested inside another node — e.g.vortex.fsstunder avortex.maskedroot, whichCascadingCompressorgenuinely produces for high-cardinality Utf8 columns — never showed up, even though the file's values really are FSST-compressed.collectEncodingsnow walks the fullArrayNodetree (root + all descendants). Since this newly makes the inspector traverse wire-supplied children it previously never touched, the walk is hardened to match the untrusted-input contract the reader module's analogous tree walks already follow:PostscriptParser.convertLayout's guard) instead of letting a crafted out-of-range index surface a rawIndexOutOfBoundsExceptionchildrenvector slots can alias the same absolute table position, so a crafted segment with aliased fan-out could otherwise drive an exponential number of visits while depth stays within the existingMAX_ENCODING_TREE_DEPTHguardDictValuesPoolCompressionIntegrationTestthat claimed the nested FSST values-pool encoding "is not visible in usedEncodings" — it now is, so the test asserts it directly.Closes #298.
Test plan
./mvnw verify -DskipTests(full reactor compile)./mvnw -pl inspector,cli -am test(unit tests, including new nested-encoding + depth/node-count/bounds-check coverage)./mvnw -pl integration -am verify -Dit.test="DictValuesPoolCompressionIntegrationTest,AlpRdCascadeSelectionIntegrationTest,NullSparseRunEndInteropIntegrationTest" -Dvortex.it.excludedGroups=(real-fileInspectorTreeconsumers)🤖 Generated with Claude Code