[Kernel] Make data skipping column lookups case-insensitive - #7579
Open
nanjeshramesh wants to merge 1 commit into
Open
[Kernel] Make data skipping column lookups case-insensitive#7579nanjeshramesh wants to merge 1 commit into
nanjeshramesh wants to merge 1 commit into
Conversation
Delta column names are case-insensitive per the protocol spec: "All column names must be unique regardless of casing." Delta Spark honors this in the data skipping path via equalsIgnoreCase, but the Java Kernel's StatsSchemaHelper built its logical-to-physical and logical-to-data-type maps keyed directly by Column, whose equals and hashCode are case-sensitive by design. A predicate like col > 5 would silently fail to match a schema column named Col, and data skipping would just not apply, no error, just a missed optimization. Column itself stays case-sensitive on purpose, it's used broadly for exact-name resolution elsewhere in the kernel, and changing its equals/hashCode would be a much bigger, riskier change than this bug calls for. Instead, StatsSchemaHelper's two internal maps are now keyed by a case-folded form of the column's path (each path segment lowercased) rather than by Column directly, matching the approach already taken on the Rust kernel side for the equivalent bug (delta-kernel-rs#2055), which case-folds the lookup key in the schema traversal rather than changing column-name equality everywhere. Delta's own protocol guarantee, that no schema can contain two leaf columns whose paths fold to the same key, means this can't introduce a collision that wasn't already prevented at the table level. Added tests to StatsSchemaHelperSuite covering: a mixed-case schema column matched by a lowercase query and vice versa, a nested column matched case-insensitively at every path segment, and confirmation that a genuinely nonexistent column is still correctly rejected. Closes delta-io#6247. Signed-off-by: Nanjesh Ramesh <nanjeshramesh7@gmail.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.
Description
Fixes the bug described in #6247: Java Kernel's
StatsSchemaHelpermatched columns for data skipping using
Column's ownequals/hashCode, which are case-sensitive by design. A predicate likecol > 5would silently fail to match a schema column namedCol,so data skipping just wouldn't apply, no error, just a missed
optimization. This is inconsistent with the protocol, which requires
column names be unique regardless of casing, and with Delta Spark,
which already handles this correctly via
equalsIgnoreCase.How
Rather than changing
Column's equality (it's intentionallycase-sensitive and used broadly elsewhere in the kernel for exact-name
resolution),
StatsSchemaHelper's two internal lookup maps are nowkeyed by a case-folded form of the column's path instead of by
Columndirectly. This scopes the case-insensitive matching to justthe stats/skipping lookups. It mirrors the fix already merged on the
Rust kernel side (delta-kernel-rs#2055), which takes the same
case-fold-the-lookup-key approach rather than touching column-name
equality globally.
This PR does NOT:
Column'sequals/hashCodeor its case-sensitivityelsewhere in the kernel
How was this patch tested?
Added test cases to
StatsSchemaHelperSuite:Value) matched by a lowercase query(
value), and the same in reverserejected (no false positives from the case-folding)
Verified each new test fails against the pre-fix code with the exact
mismatch the issue describes, and passes with the fix. Ran the full
skippingpackage test suite (15 tests,StatsSchemaHelperSuite+DataSkippingUtilsSuite), all passing.Does this PR introduce any user-facing changes?
No public API or on-disk format changes. Behavior change only: data
skipping now applies correctly for predicates that reference a column
using different casing than the schema, matching Delta Spark's
existing behavior.