github-actions[bot] commented on code in PR #67888:
URL: https://github.com/apache/doris/pull/67888#discussion_r4002086171
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/OneRangePartitionEvaluator.java:
##########
@@ -527,59 +526,47 @@ private EvaluateRangeResult
intersectSlotRange(EvaluateRangeResult originResult,
}
private EvaluateRangeResult determinateRangeOfOtherType(
- EvaluateRangeResult context, List<Literal> partitionBound, boolean
isLowerBound) {
+ EvaluateRangeResult context, List<Literal> partitionBound, boolean
isLowerBound,
+ Map<Expression, ColumnRange> defaultColumnRanges) {
if (context.result instanceof Literal) {
return context;
}
- Slot qualifiedSlot = null;
- ColumnRange qualifiedRange = null;
+ LexicographicBoundState boundState = new LexicographicBoundState(
+ partitionBound, isLowerBound, partitionSlots.size());
for (int i = 0; i < partitionSlotTypes.size(); i++) {
PartitionSlotType partitionSlotType = partitionSlotTypes.get(i);
Slot slot = partitionSlots.get(i);
- if (!context.columnRanges.containsKey(slot)) {
+ ColumnRange columnRange = context.columnRanges.containsKey(slot)
Review Comment:
[P1] Do not expose default-only ranges to `NOT`
A reduced failing filter/tree after the pruning-specific Date-IN rewrite is:
```text
Filter NOT(OR(c in day-2 range, c in day-3 range))
Scan p: [(1,10,2020-01-01), (2,20,2020-01-04))
```
This is reachable from `NOT(date(c) IN ('2020-01-02','2020-01-03'))`: the
multi-option IN survives normal optimization, then
`PredicateRewriteForPartitionPrune.visitInPredicate` creates the `Or(And(...),
And(...))` beneath `Not`. For expanded `a=1`, each inner `And` has a predicate
range only for `c`; this fallback walks missing singleton `a`, reads
default-only `b >= 10`, and `replaceExprRange` adds `b` to the child result.
`visitOr` retains it, then `visitNot` computes `default(b) ∩
complement(default(b))`, making both boundary inputs `FALSE` and dropping the
partition even though `(1,11,'2020-01-01')` satisfies the filter. Please use
defaults only to recover folded-away equal-prefix coordinates; do not insert a
missing first-unresolved `OTHER` range into the predicate result (or track
provenance), and add this negated multi-date-IN regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/OneRangePartitionEvaluator.java:
##########
@@ -748,49 +735,38 @@ private List<Map<Slot, PartitionSlotInput>>
computeSinglePartitionValueInputs()
private List<Map<Slot, PartitionSlotInput>>
commonComputeOnePartitionInputs() {
List<Map<Slot, PartitionSlotInput>> onePartitionInputs =
Lists.newArrayListWithCapacity(inputs.size());
for (List<Expression> input : inputs) {
- boolean previousIsLowerBoundLiteral = true;
- boolean previousIsUpperBoundLiteral = true;
+ LexicographicBoundState lowerState = new LexicographicBoundState(
+ lowers, true, partitionSlots.size());
+ LexicographicBoundState upperState = new LexicographicBoundState(
+ uppers, false, partitionSlots.size());
Builder<Slot, PartitionSlotInput> slotToInputs =
ImmutableMap.builderWithExpectedSize(16);
for (int i = 0; i < partitionSlots.size(); ++i) {
Slot partitionSlot = partitionSlots.get(i);
// partitionSlot will be replaced to this expression
Expression expression = input.get(i);
- ColumnRange slotRange = null;
+ ColumnRange slotRange;
PartitionSlotType partitionSlotType =
partitionSlotTypes.get(i);
if (expression instanceof Literal) {
// const or expanded range
slotRange = ColumnRange.singleton((Literal) expression);
- if (!expression.equals(lowers.get(i))) {
- previousIsLowerBoundLiteral = false;
- }
- if (!expression.equals(uppers.get(i))) {
- previousIsUpperBoundLiteral = false;
- }
+ lowerState.observeLiteral(expression, i);
+ upperState.observeLiteral(expression, i);
} else {
- // un expanded range
+ // The first unresolved column carries every still-active
lexicographic bound.
+ // Once that column can diverge, every suffix column must
remain unbounded.
switch (partitionSlotType) {
case RANGE:
- boolean isLastPartitionColumn = i + 1 ==
partitionSlots.size();
- BoundType rightBoundType = isLastPartitionColumn
- ? BoundType.OPEN : BoundType.CLOSED;
- slotRange = ColumnRange.range(
- lowers.get(i), BoundType.CLOSED,
uppers.get(i), rightBoundType);
- break;
case OTHER:
- if (previousIsLowerBoundLiteral) {
- slotRange = ColumnRange.atLeast(lowers.get(i));
- } else if (previousIsUpperBoundLiteral) {
- slotRange =
ColumnRange.lessThen(uppers.get(i));
- } else {
- // unknown range
- slotRange = ColumnRange.all();
- }
+ slotRange =
lowerState.constrainFirstUnresolvedColumn(ColumnRange.all(), i);
+ slotRange =
upperState.constrainFirstUnresolvedColumn(slotRange, i);
break;
+ case CONST:
default:
- throw new AnalysisException("Unknown partition
slot type: " + partitionSlotType);
+ // A CONST input should always be a literal. Keep
an unexpected shape conservative.
Review Comment:
[P2] Fail loudly for impossible partition-slot states
`PartitionRangeExpander` guarantees that a `CONST` input is a literal, so
reaching this branch means the evaluator/expander invariant is broken.
Returning `ColumnRange.all()` hides that failure and silently disables pruning;
the changed `default: return context` above does the same for an unhandled enum
value. The surrounding code and the previous implementation throw
`AnalysisException` for these states, and the Doris review contract requires
invariant violations to fail loudly. Please restore a throw/precondition for
the nonliteral `CONST` case and for unknown slot types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
