yujun777 commented on code in PR #69921:
URL: https://github.com/apache/doris/pull/69921#discussion_r4440859518
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/OrToIn.java:
##########
@@ -241,12 +250,105 @@ private boolean independentConjunct(int idx,
List<Expression> conjuncts) {
return true;
}
+ /**
+ * Whether conjuncts.get(idx) may contribute an "X = literal" / "X IN
(literals)" candidate.
+ *
+ * Replace mode may substitute the inferred In-predicate for the original
OR, so it keeps the
+ * historical requirement that the candidate does not share input slots
with sibling conjuncts.
+ *
+ * Extract mode never drops the original OR, it only appends "X IN (union
of literals)". When X is a
+ * plain slot (deterministic, evaluated once per row), every disjunct
implies its own top-level
+ * "X = c" / "X IN (...)", so the whole OR implies "X IN (union)" no
matter whether X is also
+ * referenced by sibling conjuncts. Under three-valued logic "X IN (union)
AND or" is equivalent to
+ * "or": or=TRUE implies X IN (union)=TRUE, and X IN (union)=FALSE implies
every disjunct is FALSE.
+ * A typical shape where the independence check blocks a useful pushdown:
+ * (spm='a' AND ...) OR (... AND spm IN ('c','d') AND ((spm='c' AND ...)
OR (spm='d' AND ...)))
+ * Here "spm IN ('c','d')" was inferred from the inner OR, which is kept
and still references spm.
+ *
+ * Config.enable_or_to_in_shared_slot_inference is the rollback switch of
exactly this relaxation:
+ * when it is false, every conjunct goes through independentConjunct() as
before.
+ */
+ private boolean isCandidateConjunct(int idx, List<Expression> conjuncts) {
+ if (mode == Mode.extractMode &&
Config.enable_or_to_in_shared_slot_inference
+ && isSlotCompareWithLiterals(conjuncts.get(idx))) {
Review Comment:
Could we update the duplicate-candidate handling together with the relaxed
independence check?
`getCandidates()` still calls `candidates.clear()` and breaks when it sees
another candidate for the same expression. The relaxation makes this reachable
for shared-slot candidates and can discard useful candidates for unrelated
slots as well.
For example, consider the expression passed to `rewriteTree(Expression)`:
```sql
(
b = 10
AND a = 1
AND ((a = 1 AND c = 100) OR (a = 2 AND c = 200))
)
OR (b = 20 AND a = 3)
```
The bottom-up rewrite of the inner OR adds `a IN (1, 2)` alongside the
retained inner OR. In the first outer disjunct, candidate collection then sees
`b = 10`, `a = 1`, and `a IN (1, 2)`. Both `a` candidates now pass the relaxed
check, but the second one clears the entire map, including `b`. With the
previous independence check, the shared `a` candidates were skipped and `b IN
(10, 20)` could still be inferred at the outer level.
Within a conjunction, compatible constraints on the same slot should be
combined by intersection: `a = 1 AND a IN (1, 2)` gives `a = 1`. Across
disjuncts, their candidate values are combined by union. Please preserve the
existing type/NULL semantics, especially when an intersection is empty, and
avoid discarding candidates for other slots. A test through
`rewriteTree(Expression)`, the entry point used by `InferInPredicateFromOr`,
would cover this nested shape. This is a rule-level inference regression; I
have not established an end-to-end query slowdown or a wrong-result case.
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -2922,6 +2922,15 @@ public static void validateMtmvCacheConfig() throws
ConfigException {
+ "upload task during the backup process. The default " + "value
is 10.")
public static int backup_upload_snapshot_batch_size = 10;
+ @ConfField(mutable = true, masterOnly = false, description = "When
inferring In-predicates from OR (OrToIn "
+ + "extract mode, used by filters and partition pruning), "
+ + "whether a `slot = literal` / `slot IN (literals)` conjunct can
be a candidate even if its slot "
+ + "is also referenced by sibling conjuncts of the same disjunct,
e.g. infer `a IN (1,2,3)` from "
+ + "`(a=1 AND b=1) OR (a IN (2,3) AND ((a=2 AND c=1) OR (a=3 AND
c=2)))`. The inferred In-predicate "
+ + "is only appended and the original OR is kept, so results do not
change. Set to false to restore "
+ + "the previous inference logic; it takes effect immediately for
queries planned afterwards.")
+ public static boolean enable_or_to_in_shared_slot_inference = true;
Review Comment:
**Non-blocking suggestion:** I would prefer removing
`enable_or_to_in_shared_slot_inference`. This rewrite does not introduce a
user-selectable semantic behavior, and adding a permanent FE config solely as a
safeguard against possible implementation bugs adds configuration and
maintenance complexity. The rule's guards and tests should establish
correctness. Removing the config is a suggestion, not a requirement for
merging; the duplicate-candidate handling above is a separate concern.
--
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]
