github-actions[bot] commented on code in PR #69921:
URL: https://github.com/apache/doris/pull/69921#discussion_r4434127461
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -2922,6 +2922,19 @@ 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 = {
Review Comment:
[P1] Supply a single string for the configuration description
`ConfigBase.ConfField.description()` is declared as `String`
(ConfigBase.java:68), but this annotation supplies a two-element array
initializer. Java rejects an array value for a scalar annotation element, so
`fe-common` and the FE cannot compile at this head. Use one description string,
or concatenate the translations, to match the existing annotation API.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/OrToIn.java:
##########
@@ -241,12 +248,59 @@ 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))) {
+ return true;
+ }
+ return independentConjunct(idx, conjuncts);
+ }
+
+ private static boolean isSlotCompareWithLiterals(Expression conjunct) {
+ if (conjunct instanceof EqualTo) {
+ EqualTo eq = (EqualTo) conjunct;
+ return (eq.left() instanceof Slot && eq.right() instanceof Literal)
+ || (eq.right() instanceof Slot && eq.left() instanceof
Literal);
+ }
+ if (conjunct instanceof InPredicate) {
+ InPredicate inPredicate = (InPredicate) conjunct;
+ if (!(inPredicate.getCompareExpr() instanceof Slot)) {
+ return false;
+ }
+ for (Expression option : inPredicate.getOptions()) {
+ if (!(option instanceof Literal)) {
+ return false;
+ }
+ }
+ return true;
+ }
+ return false;
+ }
+
private Map<Expression, Set<Literal>> getCandidates(Expression disjunct) {
List<Expression> conjuncts =
ExpressionUtils.extractConjunction(disjunct);
Map<Expression, Set<Literal>> candidates = new LinkedHashMap<>();
// collect candidates from the first disjunction
for (int idx = 0; idx < conjuncts.size(); idx++) {
- if (!independentConjunct(idx, conjuncts)) {
+ if (!isCandidateConjunct(idx, conjuncts)) {
Review Comment:
[P2] Preserve unrelated candidates when shared-slot constraints repeat
The relaxed check makes the existing duplicate-slot bailout reachable and
can remove a pushdown that worked before. For example, with nonunique `t.b` and
no MV substitution:
```sql
SELECT * FROM (
SELECT b AS b, SUM(x) AS a, SUM(y) AS c FROM t GROUP BY b
) q
WHERE (a=1 AND b=10)
OR (a IN (2,3,4) AND b=1000000
AND ((a=2 AND c=1) OR (a=3 AND c=2)));
```
The explicit aliases give all outer slots qualifier `q`, so the earlier
single-table extractor only reproduces the whole OR. After alias forwarding the
relevant tree is `Filter(OR) -> Aggregate(group by b) -> Scan`. The inner
`rewriteTree` adds `a IN (2,3)` beside the existing `a IN (2,3,4)`. Both are
now eligible, so the second triggers `candidates.clear(); break` at lines
354-355, deleting the entire branch map, including `b`. Legacy independence
skipped both shared a-INs and inferred `b IN (10,1000000)`, which could push
below Aggregate. The enabled path loses that selective scan/aggregation filter;
later simplification has no subsequent OrToIn pass to recover it. Keep or
combine a necessary constraint for the repeated slot without discarding
candidates for other slots, and cover this overlapping-IN case.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/OrToIn.java:
##########
@@ -241,12 +248,59 @@ 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))) {
+ return true;
+ }
+ return independentConjunct(idx, conjuncts);
+ }
+
+ private static boolean isSlotCompareWithLiterals(Expression conjunct) {
+ if (conjunct instanceof EqualTo) {
+ EqualTo eq = (EqualTo) conjunct;
+ return (eq.left() instanceof Slot && eq.right() instanceof Literal)
Review Comment:
[P1] Keep nullable ARRAY comparisons out of this relaxation
`ArrayLiteral` also satisfies `Literal`, so this now rewrites the following
reduced filter (literal sides typed as ARRAY<INT>):
```text
Filter[(((a=[1] AND size(a)>k) OR a=[2]) IS NULL)]
Scan[a nullable ARRAY<INT>, k]
```
`isCandidateConjunct` accepts the shared `a=[1]` and
`candidatesToFinalResult` inserts `a IN ([1],[2]) AND original_OR` inside `IS
NULL`; the old independence check left this OR alone. ARRAY IN dispatches to
the existing `FunctionCollectionIn`, which disables default null propagation
and unwraps the left nullable column without merging its null map. For `a=NULL`
with the normal empty-array payload, the added IN returns non-null FALSE while
the original equalities/OR return NULL. Consequently `IS NULL` changes from
TRUE to FALSE and the row is dropped, despite retaining the original OR.
Restrict the new candidates to comparison types with matching IN semantics, or
fix and cover `collection_in`'s left-NULL handling before admitting arrays.
--
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]
