Resolves #1674: Lucene scans without queries

FoundationDB · Jun 3, 2022 · c605c18 · c605c18
1 parent d4f3dad
commit c605c18
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 0 deletions.
diff --git a/...record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LucenePlanner.java b/...record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LucenePlanner.java
@@ -100,6 +100,10 @@ private ScoredPlan planLucene(@Nonnull CandidateScan candidateScan,
             if (!groupingMatch.getType().equals((QueryToKeyMatcher.MatchType.EQUALITY))) {
                 return null;
             }
+            if (filterMask.allSatisfied()) {
+                // If filter is only group predicates, can skip trying to find non-trivial Lucene scan.
+                return null;
+            }
             groupingComparisons = new ScanComparisons(groupingMatch.getEqualityComparisons(), Collections.emptySet());
         } else {
             groupingComparisons = ScanComparisons.EMPTY;
@@ -257,6 +261,7 @@ private LuceneQueryClause getQueryForAndOr(@Nonnull LucenePlanState state, @Nonn
         if (filterMask != null && filterMask.getUnsatisfiedFilters().isEmpty()) {
             filterMask.setSatisfied(true);
         }
+        // Don't do Lucene scan if none are satisfied, though.
         if (childClauses.isEmpty()) {
             return null;
         }

diff --git a/...d-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueryTest.java b/...d-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueryTest.java
@@ -94,6 +94,7 @@
 import static com.apple.foundationdb.record.query.plan.match.PlanMatchers.primaryKeyDistinct;
 import static com.apple.foundationdb.record.query.plan.match.PlanMatchers.scan;
 import static com.apple.foundationdb.record.query.plan.match.PlanMatchers.typeFilter;
+import static com.apple.foundationdb.record.query.plan.match.PlanMatchers.unbounded;
 import static com.apple.foundationdb.record.query.plan.match.PlanMatchers.unorderedUnion;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.aMapWithSize;
@@ -905,4 +906,39 @@ void covering() throws Exception {
         }
     }
 
+    @Test
+    void fullGroupScan() throws Exception {
+        try (FDBRecordContext context = openContext()) {
+            openRecordStore(context, md -> {
+                md.removeIndex(MAP_AND_FIELD_ON_LUCENE_INDEX.getName());
+            }, SIMPLE_TEXT_SUFFIXES);
+            QueryComponent groupFilter = Query.field("entry").oneOfThem().matches(Query.field("key").equalsValue("a"));
+            RecordQuery query = RecordQuery.newBuilder()
+                    .setRecordType(MAP_DOC)
+                    .setFilter(groupFilter)
+                    .build();
+            RecordQueryPlan plan = planner.plan(query);
+            Matcher<RecordQueryPlan> matcher = filter(groupFilter, typeFilter(equalTo(Collections.singleton(TextIndexTestUtils.MAP_DOC)), scan(unbounded())));
+            assertThat(plan, matcher);
+        }
+
+        try (FDBRecordContext context = openContext()) {
+            openRecordStore(context, md -> {
+                md.removeIndex(MAP_AND_FIELD_ON_LUCENE_INDEX.getName());
+                md.removeIndex(MAP_ON_LUCENE_INDEX.getName());
+                md.addIndex(MAP_DOC, new Index("GroupedMap", new GroupingKeyExpression(concat(field("group"), mainExpression), 1), LuceneIndexTypes.LUCENE));
+            }, SIMPLE_TEXT_SUFFIXES);
+            QueryComponent groupFilter = Query.and(
+                    Query.field("group").equalsValue(1L),
+                    Query.field("entry").oneOfThem().matches(Query.field("key").equalsValue("a")));
+            RecordQuery query = RecordQuery.newBuilder()
+                    .setRecordType(MAP_DOC)
+                    .setFilter(groupFilter)
+                    .build();
+            RecordQueryPlan plan = planner.plan(query);
+            Matcher<RecordQueryPlan> matcher = filter(groupFilter, typeFilter(equalTo(Collections.singleton(TextIndexTestUtils.MAP_DOC)), scan(unbounded())));
+            assertThat(plan, matcher);
+        }
+    }
+
 }