github-actions[bot] commented on code in PR #68687:
URL: https://github.com/apache/doris/pull/68687#discussion_r4152479970
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/source/LanceScalarIndexPlanner.java:
##########
@@ -80,26 +110,199 @@ static Plan plan(LanceTableMetadata metadata, List<Expr>
pushedConjuncts,
return selected;
}
- private static Set<Integer> collectFilterFields(LanceTableMetadata
metadata, List<Expr> pushedConjuncts) {
- Set<SlotRef> slots = new HashSet<>();
- pushedConjuncts.forEach(expr -> collectDriverSlots(expr, slots));
+ private static Set<Integer> collectFilterFields(LanceTableMetadata
metadata,
+ List<Expr> pushedConjuncts, IndexType indexType) {
Set<Integer> fields = new HashSet<>();
+ for (Expr expr : pushedConjuncts) {
+ fields.addAll(collectDriverFields(metadata, expr, indexType,
false));
+ }
+ return fields;
+ }
+
+ private static Set<Integer> collectDriverFields(LanceTableMetadata
metadata, Expr expr,
+ IndexType indexType, boolean requireExact) {
+ Set<Integer> fields = new HashSet<>();
+ if (expr instanceof CompoundPredicate) {
+ CompoundPredicate.Operator op = ((CompoundPredicate) expr).getOp();
+ if (op == CompoundPredicate.Operator.NOT) {
+ return fields;
+ }
+ boolean exactBranches = requireExact || op ==
CompoundPredicate.Operator.OR;
+ Set<Integer> left = collectDriverFields(metadata,
expr.getChild(0), indexType, exactBranches);
+ Set<Integer> right = collectDriverFields(metadata,
expr.getChild(1), indexType, exactBranches);
+ if (op == CompoundPredicate.Operator.AND) {
+ // A refine-only conjunct anywhere below OR makes native
reject the
+ // union, even if its sibling could independently drive this
index.
+ if (requireExact && (left.isEmpty() || right.isEmpty())) {
Review Comment:
[P2] Reject OR branches with an unindexed conjunct before grouping the
segment. With only `key` indexed, `(key = 1 AND other = 3) OR (key = 2 AND
other = 4)` gives both AND branches nonempty field sets here, so FE makes one
task covering every fragment in the `key` segment. Lance treats each `other`
equality as a refine expression and rejects the OR index query, so lance-c
falls back to a full scan on one BE. Check exact eligibility against the
selected index, or keep fragment splits for this shape; cover distinct `other`
values in a multi-fragment test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/source/LanceScalarIndexPlanner.java:
##########
@@ -80,26 +110,199 @@ static Plan plan(LanceTableMetadata metadata, List<Expr>
pushedConjuncts,
return selected;
}
- private static Set<Integer> collectFilterFields(LanceTableMetadata
metadata, List<Expr> pushedConjuncts) {
- Set<SlotRef> slots = new HashSet<>();
- pushedConjuncts.forEach(expr -> collectDriverSlots(expr, slots));
+ private static Set<Integer> collectFilterFields(LanceTableMetadata
metadata,
+ List<Expr> pushedConjuncts, IndexType indexType) {
Set<Integer> fields = new HashSet<>();
+ for (Expr expr : pushedConjuncts) {
+ fields.addAll(collectDriverFields(metadata, expr, indexType,
false));
+ }
+ return fields;
+ }
+
+ private static Set<Integer> collectDriverFields(LanceTableMetadata
metadata, Expr expr,
+ IndexType indexType, boolean requireExact) {
+ Set<Integer> fields = new HashSet<>();
+ if (expr instanceof CompoundPredicate) {
+ CompoundPredicate.Operator op = ((CompoundPredicate) expr).getOp();
+ if (op == CompoundPredicate.Operator.NOT) {
+ return fields;
+ }
+ boolean exactBranches = requireExact || op ==
CompoundPredicate.Operator.OR;
+ Set<Integer> left = collectDriverFields(metadata,
expr.getChild(0), indexType, exactBranches);
+ Set<Integer> right = collectDriverFields(metadata,
expr.getChild(1), indexType, exactBranches);
+ if (op == CompoundPredicate.Operator.AND) {
+ // A refine-only conjunct anywhere below OR makes native
reject the
+ // union, even if its sibling could independently drive this
index.
+ if (requireExact && (left.isEmpty() || right.isEmpty())) {
+ return fields;
+ }
+ left.addAll(right);
+ } else {
+ // Sharing a slot is insufficient: both OR branches must
actually be
+ // indexable. A suffix LIKE, for example, cannot supply BTree
candidates.
+ left.retainAll(right);
+ }
+ return left;
+ }
+ if (!isPositiveIndexLeaf(expr, indexType, requireExact)) {
+ return fields;
+ }
+ Set<SlotRef> slots = new HashSet<>();
+ expr.collect(SlotRef.class, slots);
for (SlotRef slot : slots) {
metadata.getLanceFieldId(slot.getColumnName()).ifPresent(fields::add);
}
return fields;
}
- private static void collectDriverSlots(Expr expr, Set<SlotRef> slots) {
- // A predicate below OR or NOT is not a necessary condition of the
whole filter.
- // Do not select its index and then force every task into a
non-indexed fallback.
+ private static boolean isPositiveIndexLeaf(Expr expr, IndexType indexType,
boolean requireExact) {
+ if (indexType == IndexType.LABEL_LIST) {
+ if (!(expr instanceof FunctionCallExpr) ||
expr.getChildren().size() != 2) {
+ return false;
+ }
+ String name = ((FunctionCallExpr) expr).getFnName().getFunction();
+ if ("array_contains".equalsIgnoreCase(name)) {
+ return expr.getChild(0) instanceof SlotRef && expr.getChild(1)
instanceof LiteralExpr;
+ }
+ return "arrays_overlap".equalsIgnoreCase(name) &&
overlapSize(expr) > 0;
+ }
+ if (expr instanceof BinaryPredicate) {
+ BinaryPredicate.Operator op = ((BinaryPredicate) expr).getOp();
+ return (op == BinaryPredicate.Operator.EQ || op ==
BinaryPredicate.Operator.GT
Review Comment:
[P2] Include pushed boolean and null-safe equality filters in segment driver
selection. `WHERE flag` on an indexed boolean column and `WHERE key <=> 5` on
an indexed key are translated to native indexable equality filters, but this
check rejects their original `SlotRef`/`EQ_FOR_NULL` forms. FE then emits one
fragment task per fragment with native indexing still enabled, repeating the
logical index search instead of searching the segment once. The previous slot
collector grouped these filters; cover both shapes with a multi-fragment index.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/source/LanceScalarIndexPlanner.java:
##########
@@ -80,26 +110,199 @@ static Plan plan(LanceTableMetadata metadata, List<Expr>
pushedConjuncts,
return selected;
}
- private static Set<Integer> collectFilterFields(LanceTableMetadata
metadata, List<Expr> pushedConjuncts) {
- Set<SlotRef> slots = new HashSet<>();
- pushedConjuncts.forEach(expr -> collectDriverSlots(expr, slots));
+ private static Set<Integer> collectFilterFields(LanceTableMetadata
metadata,
+ List<Expr> pushedConjuncts, IndexType indexType) {
Set<Integer> fields = new HashSet<>();
+ for (Expr expr : pushedConjuncts) {
+ fields.addAll(collectDriverFields(metadata, expr, indexType,
false));
+ }
+ return fields;
+ }
+
+ private static Set<Integer> collectDriverFields(LanceTableMetadata
metadata, Expr expr,
+ IndexType indexType, boolean requireExact) {
+ Set<Integer> fields = new HashSet<>();
+ if (expr instanceof CompoundPredicate) {
+ CompoundPredicate.Operator op = ((CompoundPredicate) expr).getOp();
+ if (op == CompoundPredicate.Operator.NOT) {
+ return fields;
+ }
+ boolean exactBranches = requireExact || op ==
CompoundPredicate.Operator.OR;
+ Set<Integer> left = collectDriverFields(metadata,
expr.getChild(0), indexType, exactBranches);
+ Set<Integer> right = collectDriverFields(metadata,
expr.getChild(1), indexType, exactBranches);
+ if (op == CompoundPredicate.Operator.AND) {
+ // A refine-only conjunct anywhere below OR makes native
reject the
+ // union, even if its sibling could independently drive this
index.
+ if (requireExact && (left.isEmpty() || right.isEmpty())) {
+ return fields;
+ }
+ left.addAll(right);
+ } else {
+ // Sharing a slot is insufficient: both OR branches must
actually be
+ // indexable. A suffix LIKE, for example, cannot supply BTree
candidates.
+ left.retainAll(right);
+ }
+ return left;
+ }
+ if (!isPositiveIndexLeaf(expr, indexType, requireExact)) {
+ return fields;
+ }
+ Set<SlotRef> slots = new HashSet<>();
+ expr.collect(SlotRef.class, slots);
for (SlotRef slot : slots) {
metadata.getLanceFieldId(slot.getColumnName()).ifPresent(fields::add);
}
return fields;
}
- private static void collectDriverSlots(Expr expr, Set<SlotRef> slots) {
- // A predicate below OR or NOT is not a necessary condition of the
whole filter.
- // Do not select its index and then force every task into a
non-indexed fallback.
+ private static boolean isPositiveIndexLeaf(Expr expr, IndexType indexType,
boolean requireExact) {
+ if (indexType == IndexType.LABEL_LIST) {
+ if (!(expr instanceof FunctionCallExpr) ||
expr.getChildren().size() != 2) {
+ return false;
+ }
+ String name = ((FunctionCallExpr) expr).getFnName().getFunction();
+ if ("array_contains".equalsIgnoreCase(name)) {
+ return expr.getChild(0) instanceof SlotRef && expr.getChild(1)
instanceof LiteralExpr;
+ }
+ return "arrays_overlap".equalsIgnoreCase(name) &&
overlapSize(expr) > 0;
+ }
+ if (expr instanceof BinaryPredicate) {
+ BinaryPredicate.Operator op = ((BinaryPredicate) expr).getOp();
+ return (op == BinaryPredicate.Operator.EQ || op ==
BinaryPredicate.Operator.GT
+ || op == BinaryPredicate.Operator.GE || op ==
BinaryPredicate.Operator.LT
+ || op == BinaryPredicate.Operator.LE)
+ && ((expr.getChild(0) instanceof SlotRef &&
expr.getChild(1) instanceof LiteralExpr)
+ || (expr.getChild(1) instanceof SlotRef &&
expr.getChild(0) instanceof LiteralExpr));
+ }
+ if (expr instanceof InPredicate) {
+ return !((InPredicate) expr).isNotIn() && expr.getChild(0)
instanceof SlotRef;
+ }
+ if (expr instanceof IsNullPredicate) {
+ return !((IsNullPredicate) expr).isNotNull() && expr.getChild(0)
instanceof SlotRef;
+ }
+ // Bitmap has no prefix-query support. A refined LIKE can drive an AND,
+ // but native's OR planner rejects branches that need a residual
recheck.
+ if (indexType != IndexType.BTREE || expr.getChildren().size() != 2
+ || !(expr.getChild(0) instanceof SlotRef) ||
!(expr.getChild(1) instanceof StringLiteral)) {
+ return false;
+ }
+ String name = expr instanceof FunctionCallExpr
+ ? ((FunctionCallExpr) expr).getFnName().getFunction() : "";
+ String prefix = ((StringLiteral) expr.getChild(1)).getStringValue();
+ if ("starts_with".equalsIgnoreCase(name)) {
+ // Unlike LIKE, every character in starts_with is literal,
including % and _.
+ return !prefix.isEmpty();
+ }
+ boolean like = (expr instanceof LikePredicate && ((LikePredicate)
expr).getOp() == LikePredicate.Operator.LIKE)
+ || "like".equalsIgnoreCase(name);
+ if (!like || prefix.isEmpty() || prefix.indexOf('\\') >= 0) {
+ return false;
+ }
+ for (int i = 0; i < prefix.length(); i++) {
+ char character = prefix.charAt(i);
+ if (character == '%' || character == '_') {
+ return i > 0 && (!requireExact || (character == '%' && i ==
prefix.length() - 1));
+ }
+ }
+ return true;
+ }
+
+ private static int overlapSize(Expr expr) {
+ if (!(expr instanceof FunctionCallExpr) || expr.getChildren().size()
!= 2
+ || !"arrays_overlap".equalsIgnoreCase(((FunctionCallExpr)
expr).getFnName().getFunction())) {
+ return 0;
+ }
+ for (int i = 0; i < 2; i++) {
+ if (expr.getChild(i) instanceof ArrayLiteral && expr.getChild(1 -
i) instanceof SlotRef) {
+ return expr.getChild(i).getChildren().size();
+ }
+ }
+ return 0;
+ }
+
+ static boolean shouldDisableFragmentIndex(List<Expr> pushedConjuncts) {
+ if (pushedConjuncts.isEmpty()) {
+ return false;
+ }
+ // Missing metadata or an ambiguous index name is not evidence against
native
+ // index use. Disable only known expensive shapes, independent of FE
discovery.
+ return exceedsExpressionBudget(pushedConjuncts)
Review Comment:
[P2] Apply this budget to the index query rather than every pushed conjunct.
With a multi-fragment `key` BTree index, `key = 1` plus 64 equalities on
unindexed columns counts as 129 FE nodes, so this condition disables scalar
indexing on every fragment. Pinned Lance keeps only the `key` equality in
`filter_plan.index_query`; the other predicates are refinements, and native's
128-node segment budget would see one node. Keep the valid driver or preserve
native index use on the fragment splits, and cover this boundary with one
indexed field and many residual conjuncts.
--
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]
