github-actions[bot] commented on code in PR #68687:
URL: https://github.com/apache/doris/pull/68687#discussion_r4150314355
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/source/LanceScalarIndexPlanner.java:
##########
@@ -61,18 +64,21 @@ static Plan plan(LanceTableMetadata metadata, List<Expr>
pushedConjuncts,
// provides a stable winner when multiple indices cover the same
number of rows.
List<LanceIndexSegmentGroup> indices = new
ArrayList<>(metadata.getIndexes());
indices.sort(java.util.Comparator.comparing(LanceIndexSegmentGroup::getName));
+ boolean splitComplementByFragment = pushedConjuncts.stream()
+ .anyMatch(LanceScalarIndexPlanner::containsComplement);
Plan selected = null;
for (LanceIndexSegmentGroup logicalIndex : indices) {
List<LanceIndexSegmentInfo> segments = logicalIndex.getSegments();
- // PR #79 supports one top-level key in BTree/Bitmap/LabelList
indices. Lance
+ // Segment-scoped scans support one top-level key in
BTree/Bitmap/LabelList indices. Lance
// performs the final typed driver selection and falls back within
the same domain.
LanceIndexSegmentInfo index = segments.get(0);
if ((index.getIndexType() != IndexType.BTREE &&
index.getIndexType() != IndexType.BITMAP
&& index.getIndexType() != IndexType.LABEL_LIST)
|| index.getFieldIds().size() != 1 ||
!filterFields.contains(index.getFieldIds().get(0))) {
continue;
}
- Plan candidate = groupFragments(metadata, segments,
visibleFragments);
+ Plan candidate = groupFragments(metadata, segments,
visibleFragments,
Review Comment:
[P2] Keep large overlap filters out of a single segment split.
`arrays_overlap(labels, [65 distinct strings])` becomes 65 `array_has` leaves
plus 64 OR nodes, while the pinned lance-c scoped planner stops at 128 nodes
and falls back with `expression_budget`. This plan groups every fragment
covered by the LabelList segment into one task, so one BE scans the full domain
and evaluates 65 membership calls per row. Use fragment splits or another
bounded plan when the native index expression cannot be used, and cover this
threshold in a regression.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/source/LanceScalarIndexPlanner.java:
##########
@@ -131,7 +162,17 @@ private static Plan groupFragments(LanceTableMetadata
metadata, List<LanceIndexS
coveredRows += Math.max(fragment.getPhysicalRows(), 0);
}
if (!fragments.isEmpty()) {
- splits.addIndexSegmentSplit(segment.getUuid(), fragments,
physicalRows);
+ // Complement selectivity is unknown. Preserve fragment-level
scheduling so a
+ // broad NOT cannot funnel all covered data through one BE.
Each task retains
+ // the same segment UUID but owns a disjoint domain, including
on fallback.
+ if (splitComplementByFragment) {
Review Comment:
[P2] Avoid repeating a segment-wide index search for every fragment.
`splitComplementByFragment` is enabled by any pushed complement, including one
on an unrelated conjunct such as `key = 1 AND other <> 0`. Every resulting task
gets the same segment UUID; lance-c evaluates that segment's key index for each
task, then applies the candidate mask to its fragment-scoped reader. F
fragments therefore repeat the full search F times and can hold F candidate
masks. Keep fragment parallelism for broad complements without repeating a
full-segment search for a positive driver, and cover a multi-fragment
mixed-conjunct case.
##########
thirdparty/vars.sh:
##########
@@ -580,11 +580,11 @@ PUGIXML_SOURCE=pugixml-1.15
PUGIXML_MD5SUM="3b894c29455eb33a40b165c6e2de5895"
# lance-c
-# Complete-segment prefilter fixes are supplied by upstream lance-c, not local
patches.
-LANCE_C_DOWNLOAD="https://codeload.github.com/lance-format/lance-c/tar.gz/9bd730add2ac70316c1d642b8459011e2dd92022";
-LANCE_C_NAME="lance-c-9bd730add2ac70316c1d642b8459011e2dd92022.tar.gz"
-LANCE_C_SOURCE="lance-c-9bd730add2ac70316c1d642b8459011e2dd92022"
-LANCE_C_MD5SUM="63851b09bf1689032579f1a094ff2f37"
+# Includes lance-c #93: scoped boolean scalar-index expressions and Substrait
label filters.
+LANCE_C_DOWNLOAD="https://codeload.github.com/lance-format/lance-c/tar.gz/cd63420bfbe27f6f0a1edcc873b9191af7d52852";
Review Comment:
[P2] Make the updated lance-c revision reach builds with an installed
archive. A complete pre-change compilation image has
`installed/lib64/liblance_c.a` and the other sentinels that let `build.sh` skip
the third-party rebuild, so it links the previous 9bd730 archive after this pin
changes to cd63420. That old scoped planner falls back for OR/IN, while the new
regression requires index searches and zero fallbacks; the unchanged C ABI
gives no link error. Record and check the installed Lance revision, or force a
rebuild when this pin changes.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/source/LanceScalarIndexPlanner.java:
##########
@@ -81,29 +87,54 @@ static Plan plan(LanceTableMetadata metadata, List<Expr>
pushedConjuncts,
}
private static Set<Integer> collectFilterFields(LanceTableMetadata
metadata, List<Expr> pushedConjuncts) {
+ Set<Integer> fields = new HashSet<>();
+ for (Expr expr : pushedConjuncts) {
+ fields.addAll(collectDriverFields(metadata, expr, false));
+ }
+ return fields;
+ }
+
+ private static Set<Integer> collectDriverFields(LanceTableMetadata
metadata, Expr expr, boolean complete) {
+ if (expr instanceof CompoundPredicate) {
+ CompoundPredicate.Operator op = ((CompoundPredicate) expr).getOp();
+ if (op == CompoundPredicate.Operator.NOT) {
+ // NOT cannot complement a pruned AND: that would discard
valid matches.
+ return collectDriverFields(metadata, expr.getChild(0), true);
+ }
+ Set<Integer> left = collectDriverFields(metadata,
expr.getChild(0), complete);
+ Set<Integer> right = collectDriverFields(metadata,
expr.getChild(1), complete);
+ if (op == CompoundPredicate.Operator.AND && !complete) {
+ left.addAll(right);
+ } else {
+ // One selected index must supply candidates for both OR
branches, and for
+ // every leaf of a negated subtree. lance-c rechecks the
complete predicate.
+ left.retainAll(right);
Review Comment:
[P2] Do not treat a shared column as proof both OR branches can use the
index. For `key = 'x' OR key LIKE '%y%'` on a multi-fragment BTree segment,
both branches contribute `key` here, so the planner chooses one grouped segment
task. The pinned Lance planner cannot derive a prefix for `%y%`, drops the OR
index query, and lance-c falls back to scanning every covered fragment in that
one task. This loses the prior per-fragment parallelism for a common predicate.
Check branch indexability before grouping, or use fragment splits for
unsupported shapes, and cover this case in a multi-fragment test.
--
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]
