[common] Do not drop a partition on contradictory partition-key equals - #9632
Open
LuciferYang wants to merge 1 commit into
Open
[common] Do not drop a partition on contradictory partition-key equals#9632LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
OnlyPartitionKeyEqualVisitor collected partition-key equals into a map, so pt = 'a' AND pt = 'b' overwrote the first literal and still reported droppable: the Flink sink would drop partition pt='b' for a predicate matching no row. Track the conflict in visitEqual and report non-droppable from visitAnd. Reaching this from SQL needs a planner that forwards the contradiction unfolded, which Calcite does not, so this hardens the contract rather than fixing an observed failure.
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.
Purpose
close #9631
OnlyPartitionKeyEqualVisitordecides whether a DELETE can be turned into a partition drop, and it collects the equality literals into aMap<String, String>keyed by partition key. Two equals on the same key overwrite each other, sopt = 'a' AND pt = 'b'reported "droppable" withpartitions()returning{pt=b}: the Flink sink would drop that partition for a predicate that matches no row.visitEqualnow notices when a key already carries a different literal, andvisitAndreports non-droppable in that case. The delete then goes down the normal row-level path, which is what every other predicate shape this visitor does not understand already gets.Worth stating plainly, because it changes how you may want to prioritize this: I could not construct a SQL statement that reaches it. Calcite folds
pt = 'a' AND pt = 'b'to FALSE beforeapplyDeleteFilterssees it, andPredicateBuilder.anddoes not merge equality predicates on its own. So the protection exists today, but it lives in the planner rather than here, and it does not cover a caller that builds the predicate directly or a future engine binding. Given the consequence is deleting a partition, keeping the decision honest inside the visitor seemed worth the six lines.Tests
OnlyPartitionKeyEqualVisitorTestcovers the three shapes: two conflicting equals on one key report non-droppable, equals on two different keys stay droppable and produce both partition values, and an equal on a non-partition column is not droppable and contributes nothing.Against the unfixed visitor the first of those fails, reporting droppable.
mvn -pl paimon-common -Dtest=OnlyPartitionKeyEqualVisitorTest,DeletePushDownVisitorTest teston JDK 8: 4 tests, 0 failures.spotless:checkandcheckstyle:checkon paimon-common are clean.