xiangfu0 commented on code in PR #19760:
URL: https://github.com/apache/pinot/pull/19760#discussion_r4193808134
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -255,6 +278,64 @@ private MutableRoaringBitmap
getDocIdsWithoutOptions(String searchQuery) {
}
}
+ @Override
+ public int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString) {
+ try {
+ Query query = buildQuery(searchQuery, optionsString);
+ int count = countWithoutMaterializing(query);
+ if (count >= 0) {
+ return count;
+ }
+ // Lucene cannot count this query from metadata. Collect instead,
reusing the query already built and the
+ // collector that carries the query-termination check, so a slow count
still honours the query timeout.
+ MutableRoaringBitmap docIds = new MutableRoaringBitmap();
+ _indexSearcher.search(query, new LuceneDocIdCollector(docIds,
_docIdTranslator));
+ return docIds.getCardinality();
+ } catch (Exception e) {
+ throw new RuntimeException(
+ "Caught exception while counting matches in the text index for
column:" + _column + " search query:"
+ + searchQuery, e);
+ }
+ }
+
+ /// Counts matches from index metadata alone, without visiting any document.
+ ///
+ /// Lucene can do this for some queries -- a single term in particular --
and reports that it cannot by
+ /// returning -1 for a leaf, in which case this returns -1 and the caller
collects instead.
+ ///
+ /// @return the match count, or -1 if any leaf cannot be counted without
materializing matches
+ @VisibleForTesting
+ int countWithoutMaterializing(Query query)
Review Comment:
`countWithoutMaterializing` bypasses `_docIdTranslator`, so a
Lucene-docs/Pinot-docs mismatch now yields a silently wrong `COUNT(*)` where
the old path failed loudly.
This is reproducible: on an index whose Lucene doc count exceeds the Pinot
segment's `numDocs` (reachable today via on-disk segments converted by
pre-#18885 Pinot after a crashed conversion retry), `getDocIds` throws
`IndexOutOfBoundsException` from `DefaultDocIdTranslator`, but this fast path
returns the inflated raw Lucene count (e.g. 8 on an index where `numDocs=4`).
That turns detectable corruption into a wrong answer.
Cheap hardening: skip the fast path (or fail) when `_indexReader.numDocs() >
numDocs`.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -255,6 +278,64 @@ private MutableRoaringBitmap
getDocIdsWithoutOptions(String searchQuery) {
}
}
+ @Override
+ public int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString) {
+ try {
+ Query query = buildQuery(searchQuery, optionsString);
+ int count = countWithoutMaterializing(query);
Review Comment:
Every non-countable query shape (phrases, booleans,
wildcard/prefix/fuzzy/regexp — common TEXT_MATCH shapes) now pays query rewrite
+ `createWeight` twice per `count(*)`: the probe on `_countSearcher` is thrown
away on `-1`, and the fallback `_indexSearcher.search` rewrites from scratch.
For e.g. `count(*) WHERE TEXT_MATCH(col, 'conn*')`, the multi-term rewrite
enumerates the terms dictionary once in `countWithoutMaterializing` and again
inside the fallback search — making these counts measurably **slower** than
before this PR.
Lucene's own `IndexSearcher#count` avoids this with a
`TotalHitCountCollector`-style design: one search, `weight.count(ctx)` probed
per leaf inside `getLeafCollector` (throwing `CollectionTerminatedException` on
countable leaves, counting collection otherwise). That gives one rewrite, one
`Weight`, per-leaf hybrid counting — and no second searcher needed. Adopting it
here would also resolve the `_countSearcher` duplication and the fallback's
bitmap materialization (see other comments).
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -67,6 +72,11 @@ public class LuceneTextIndexReader implements
TextIndexReader {
private final IndexReader _indexReader;
private final Directory _indexDirectory;
private final IndexSearcher _indexSearcher;
+ /// Used only by [#getNumMatchingDocs]. Deliberately never query-cached: the
caching policy counts
+ /// every Weight created, so probing on the shared searcher and then falling
back to a search would
+ /// register two uses for one query and promote entries into
[org.apache.lucene.search.LRUQueryCache]
+ /// at twice the intended rate -- the heap behaviour the query cache is
disabled by default to avoid.
+ private final IndexSearcher _countSearcher;
Review Comment:
A few issues with the dedicated `_countSearcher`:
- In the default config (query cache already disabled) it's a byte-for-byte
duplicate of `_indexSearcher`.
- For users who set `enableQueryCache=true`, it unconditionally forfeits
cached counts (Lucene 9.12's `CachingWrapperWeight.count` serves cached
`CacheAndCount` entries), so repeated counts get strictly slower than before
the PR.
- The justifying comment misstates the mechanism:
`UsageTrackingQueryCachingPolicy.onUse` fires per *use* of a weight, not per
`createWeight`.
- Two searcher fields must now be kept in sync across both constructors for
any future searcher-level setting, or the count path silently diverges from the
filter path.
Simpler: `_countSearcher = config.isEnableQueryCache() ? uncachedSearcher :
_indexSearcher` — or eliminate the field entirely via the single-pass
counting-collector design (see comment on `getNumMatchingDocs`), which creates
exactly one `Weight`.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -255,6 +278,64 @@ private MutableRoaringBitmap
getDocIdsWithoutOptions(String searchQuery) {
}
}
+ @Override
+ public int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString) {
+ try {
+ Query query = buildQuery(searchQuery, optionsString);
+ int count = countWithoutMaterializing(query);
+ if (count >= 0) {
+ return count;
+ }
+ // Lucene cannot count this query from metadata. Collect instead,
reusing the query already built and the
+ // collector that carries the query-termination check, so a slow count
still honours the query timeout.
+ MutableRoaringBitmap docIds = new MutableRoaringBitmap();
+ _indexSearcher.search(query, new LuceneDocIdCollector(docIds,
_docIdTranslator));
+ return docIds.getCardinality();
+ } catch (Exception e) {
+ throw new RuntimeException(
+ "Caught exception while counting matches in the text index for
column:" + _column + " search query:"
+ + searchQuery, e);
+ }
+ }
+
+ /// Counts matches from index metadata alone, without visiting any document.
+ ///
+ /// Lucene can do this for some queries -- a single term in particular --
and reports that it cannot by
+ /// returning -1 for a leaf, in which case this returns -1 and the caller
collects instead.
+ ///
+ /// @return the match count, or -1 if any leaf cannot be counted without
materializing matches
+ @VisibleForTesting
+ int countWithoutMaterializing(Query query)
+ throws IOException {
+ Weight weight = _countSearcher.createWeight(_countSearcher.rewrite(new
ConstantScoreQuery(query)),
+ ScoreMode.COMPLETE_NO_SCORES, 1f);
+ int total = 0;
+ for (LeafReaderContext leaf : _indexReader.leaves()) {
+ int leafCount = weight.count(leaf);
+ if (leafCount == -1) {
+ return -1;
+ }
+ total += leafCount;
+ }
+ return total;
+ }
+
+ /// Parses a search string into a Lucene query, honouring the options string
when one is given.
+ ///
+ /// Shared by the doc-id and the count paths so that a `count(*)` and the
filter it counts can never
Review Comment:
This javadoc claims the method is "Shared by the doc-id and the count
paths", but the doc-id path never calls it — `getDocIds(String, String)`
performs the same options dispatch independently. So the invariant the comment
promises (count and filter can never interpret a query differently) is not
actually enforced: a maintainer could edit options handling here, trust the
comment, and skip `getDocIds` — shipping exactly the count-vs-filter divergence
this comment says is impossible. `LuceneTextIndexCountTest` can't catch that,
because the fallback uses `buildQuery` too, making the comparison a tautology.
Suggest routing `getDocIds(String, String)` through `buildQuery(searchQuery,
optionsString)` (which would also collapse
`getDocIdsWithOptions`/`getDocIdsWithoutOptions`), or correcting the comments
on both overloads (the 1-arg overload's comment also says it applies "options",
which it doesn't).
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -255,6 +278,64 @@ private MutableRoaringBitmap
getDocIdsWithoutOptions(String searchQuery) {
}
}
+ @Override
+ public int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString) {
+ try {
+ Query query = buildQuery(searchQuery, optionsString);
+ int count = countWithoutMaterializing(query);
+ if (count >= 0) {
+ return count;
+ }
+ // Lucene cannot count this query from metadata. Collect instead,
reusing the query already built and the
Review Comment:
The fallback still materializes the full doc-id bitmap plus a per-hit
`DocIdTranslator` lookup just to read `getCardinality()` — the exact per-match
cost this PR exists to remove — and is a third in-class copy of the bitmap +
`LuceneDocIdCollector` + search idiom. A count-only `LeafCollector` keeping the
same `QueryThreadContext.checkTerminationAndSampleUsagePeriodically` check but
incrementing an `int` preserves the timeout behaviour and skips both;
translation is cardinality-invariant (1:1 mapping).
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TextMatchFilterOperator.java:
##########
@@ -79,10 +79,13 @@ public boolean canOptimizeCount() {
@Override
public int getNumMatchingDocs() {
if (_textIndexReader.isMultiColumn()) {
+ // TODO: give MultiColumnTextIndexReader the same count short-circuit.
return ((MultiColumnTextIndexReader)
_textIndexReader).getDocIds(_column, _predicate.getValue(),
_predicate.getOptions()).getCardinality();
} else {
Review Comment:
Rather than an operator-side TODO, a two-line default method on
`MultiColumnTextIndexReader` — `getNumMatchingDocs(column, query, options)`
defaulting to `getDocIds(...).getCardinality()` — would put both branches at
the same altitude, so completing the TODO only touches the reader, not this
operator again.
Also a latent landmine worth noting: `MultiColumnLuceneTextIndexReader`
inherits the new single-column `getNumMatchingDocs` default, whose delegate
`getDocIds` throws `UnsupportedOperationException("Multi-column text index
requires column name to query!")` if any future caller invokes it generically.
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java:
##########
@@ -0,0 +1,166 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.segment.index.readers.text;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import org.apache.commons.io.FileUtils;
+import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
+import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
+import org.apache.pinot.segment.spi.index.TextIndexConfig;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+
+/// Pins the invariant behind the `count(*)` short-circuit:
[LuceneTextIndexReader#getNumMatchingDocs] must
+/// return exactly what counting the materialized doc ids would return, for
every query shape.
+///
+/// The short-circuit reads a count from Lucene's index metadata where it can
and falls back to materializing
+/// doc ids where it cannot, so the two paths have to agree or a `count(*)`
would silently disagree with the
+/// rows its own filter returns.
+public class LuceneTextIndexCountTest {
+ private static final File INDEX_DIR =
Review Comment:
`INDEX_DIR` is a fixed, non-unique path under the shared system temp
directory. Two concurrent executions on the same host (parallel CI executors or
surefire forks sharing `/tmp`) race: one JVM's `@BeforeClass` `deleteDirectory`
removes the other's live mmapped index mid-test, causing intermittent
`NoSuchFileException`/wrong-count flakes. Suggest appending a UUID/nanoTime
suffix as other Pinot index tests do.
Minor, while here: the class javadoc describes behavior but omits a
thread-safety statement (repo convention for new classes), which is relevant
given the static shared `INDEX_DIR`.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -255,6 +278,64 @@ private MutableRoaringBitmap
getDocIdsWithoutOptions(String searchQuery) {
}
}
+ @Override
+ public int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString) {
+ try {
+ Query query = buildQuery(searchQuery, optionsString);
+ int count = countWithoutMaterializing(query);
+ if (count >= 0) {
+ return count;
+ }
+ // Lucene cannot count this query from metadata. Collect instead,
reusing the query already built and the
+ // collector that carries the query-termination check, so a slow count
still honours the query timeout.
+ MutableRoaringBitmap docIds = new MutableRoaringBitmap();
+ _indexSearcher.search(query, new LuceneDocIdCollector(docIds,
_docIdTranslator));
+ return docIds.getCardinality();
+ } catch (Exception e) {
+ throw new RuntimeException(
+ "Caught exception while counting matches in the text index for
column:" + _column + " search query:"
+ + searchQuery, e);
+ }
+ }
+
+ /// Counts matches from index metadata alone, without visiting any document.
+ ///
+ /// Lucene can do this for some queries -- a single term in particular --
and reports that it cannot by
+ /// returning -1 for a leaf, in which case this returns -1 and the caller
collects instead.
+ ///
+ /// @return the match count, or -1 if any leaf cannot be counted without
materializing matches
+ @VisibleForTesting
+ int countWithoutMaterializing(Query query)
+ throws IOException {
+ Weight weight = _countSearcher.createWeight(_countSearcher.rewrite(new
ConstantScoreQuery(query)),
Review Comment:
Two notes on this line:
1. The `ConstantScoreQuery` wrap before rewrite looks redundant next to
`ScoreMode.COMPLETE_NO_SCORES`, but it's load-bearing — it mirrors Lucene's
`IndexSearcher#count` recipe for producing count-friendly weights. Nothing in
the code says so, and a future cleanup that removes the "redundant" wrap
wouldn't fail any test: some rewritten shapes would just stop producing
countable weights and silently degrade to full collection (which
`testFallbackPathIsExercised` is explicitly designed not to catch). Worth one
comment line pointing at `IndexSearcher#count`.
2. Minor: the metadata-count probe (rewrite + `createWeight` + per-leaf
`Weight.count`) runs outside Pinot's query-termination and resource-usage
accounting, so the in-code claim that a slow count "still honours the query
timeout" is only true of the fallback half. A regexp or leading-wildcard count
over a huge terms dictionary does bounded but non-trivial per-leaf term
enumeration during rewrite that the query killer can't interrupt. Low severity
— the old path's rewrite inside `search()` was equally unchecked — but the
probe doubles that unaccounted work for non-countable shapes.
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java:
##########
@@ -0,0 +1,166 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.segment.index.readers.text;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import org.apache.commons.io.FileUtils;
+import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
+import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
+import org.apache.pinot.segment.spi.index.TextIndexConfig;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+
+/// Pins the invariant behind the `count(*)` short-circuit:
[LuceneTextIndexReader#getNumMatchingDocs] must
+/// return exactly what counting the materialized doc ids would return, for
every query shape.
+///
+/// The short-circuit reads a count from Lucene's index metadata where it can
and falls back to materializing
+/// doc ids where it cannot, so the two paths have to agree or a `count(*)`
would silently disagree with the
+/// rows its own filter returns.
+public class LuceneTextIndexCountTest {
+ private static final File INDEX_DIR =
+ new File(FileUtils.getTempDirectory(),
LuceneTextIndexCountTest.class.getSimpleName());
+ private static final String COLUMN = "body";
+ /// Every document carries `sentinelall` so there is a term matching all
documents. `to` cannot serve that
+ /// role: it is in Pinot's default English stop-word set and matches nothing.
+ private static final String[] DOCS = {
+ "failed to place order for user alice sentinelall",
+ "failed to charge card for user bob sentinelall",
+ "connection refused by payment service sentinelall",
+ "connection established to cache service sentinelall",
+ "order confirmation email sent to carol sentinelall",
+ "cache miss while loading order details sentinelall",
+ "payment accepted for order 4815 sentinelall",
+ "unrelated housekeeping log line sentinelall"
+ };
+
+ @BeforeClass
+ public void setUp()
+ throws IOException {
+ FileUtils.deleteDirectory(INDEX_DIR);
+ FileUtils.forceMkdir(INDEX_DIR);
+ TextIndexConfig config = new TextIndexConfigBuilder().build();
+ try (LuceneTextIndexCreator creator = new LuceneTextIndexCreator(COLUMN,
INDEX_DIR, true, false, null, null,
+ config)) {
+ for (String doc : DOCS) {
+ creator.add(doc);
+ }
+ creator.seal();
Review Comment:
This fixture is always a single-leaf index: `LuceneTextIndexCreator.seal()`
calls `forceMerge(1)`. That means `countWithoutMaterializing`'s only
non-trivial logic — cross-leaf accumulation and the mid-loop `return -1` — is
never exercised by any test. A regression in the leaf loop (`break`/`continue`
instead of `return -1`, or dropped accumulation) would pass this whole suite
and then return a partial sum instead of falling back on any multi-leaf index,
producing a silently short `count(*)`.
Suggest adding a fixture built via `IndexWriter` with commits between
batches (no force-merge) so `leaves() > 1`, asserting both the summed count and
the `-1` bail.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -227,26 +241,35 @@ public MutableRoaringBitmap getDocIds(String searchQuery,
@Nullable String optio
return getDocIdsWithoutOptions(searchQuery);
}
+ /// Parses a search string into a Lucene query, applying the configured
parser and options.
+ ///
+ /// Shared by the doc-id and the count paths so that a `count(*)` and the
filter it counts can never
+ /// interpret the same query differently.
+ private Query buildQuery(String searchQuery)
Review Comment:
The count short-circuit lives as a private method in one of four
near-identical Lucene readers; `RealtimeLuceneTextIndex`,
`MultiColumnLuceneTextIndexReader`, and `MultiColumnRealtimeLuceneTextIndex`
keep materializing, so the optimization is largely invisible on realtime-heavy
and multi-column tables (a hybrid table's `count(*)` stays dominated by
consuming segments' full bitmap builds, plus per-doc stored-field lookups in
`RealtimeLuceneTextIndex.getPinotDocIds`). A static
`LuceneTextIndexUtils.countWithoutMaterializing(IndexSearcher, Query)` would
let all four readers adopt it in a few lines, since a count never needs doc-id
translation — the very duplication the TODO at the bottom of this file
documents.
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java:
##########
@@ -0,0 +1,166 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.segment.index.readers.text;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import org.apache.commons.io.FileUtils;
+import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
+import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
+import org.apache.pinot.segment.spi.index.TextIndexConfig;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+
+/// Pins the invariant behind the `count(*)` short-circuit:
[LuceneTextIndexReader#getNumMatchingDocs] must
+/// return exactly what counting the materialized doc ids would return, for
every query shape.
+///
+/// The short-circuit reads a count from Lucene's index metadata where it can
and falls back to materializing
+/// doc ids where it cannot, so the two paths have to agree or a `count(*)`
would silently disagree with the
+/// rows its own filter returns.
+public class LuceneTextIndexCountTest {
+ private static final File INDEX_DIR =
+ new File(FileUtils.getTempDirectory(),
LuceneTextIndexCountTest.class.getSimpleName());
+ private static final String COLUMN = "body";
+ /// Every document carries `sentinelall` so there is a term matching all
documents. `to` cannot serve that
+ /// role: it is in Pinot's default English stop-word set and matches nothing.
+ private static final String[] DOCS = {
+ "failed to place order for user alice sentinelall",
+ "failed to charge card for user bob sentinelall",
+ "connection refused by payment service sentinelall",
+ "connection established to cache service sentinelall",
+ "order confirmation email sent to carol sentinelall",
+ "cache miss while loading order details sentinelall",
+ "payment accepted for order 4815 sentinelall",
+ "unrelated housekeeping log line sentinelall"
+ };
+
+ @BeforeClass
+ public void setUp()
+ throws IOException {
+ FileUtils.deleteDirectory(INDEX_DIR);
+ FileUtils.forceMkdir(INDEX_DIR);
+ TextIndexConfig config = new TextIndexConfigBuilder().build();
+ try (LuceneTextIndexCreator creator = new LuceneTextIndexCreator(COLUMN,
INDEX_DIR, true, false, null, null,
+ config)) {
+ for (String doc : DOCS) {
+ creator.add(doc);
+ }
+ creator.seal();
+ }
+ }
+
+ @AfterClass
+ public void tearDown()
+ throws IOException {
+ FileUtils.deleteDirectory(INDEX_DIR);
+ }
+
+ @DataProvider(name = "queries")
+ public Object[][] queries() {
+ return new Object[][]{
+ // Single terms: the shape Lucene can count from metadata alone, i.e.
the case the optimization targets.
+ {"order", null},
+ {"connection", null},
+ {"housekeeping", null},
+ // No match, and a term in every document: the boundary cases.
+ {"nonexistentterm", null},
+ {"sentinelall", null},
+ // A stop word, which the default analyzer drops entirely.
+ {"to", null},
+ // Boolean, phrase and multi-term shapes, which generally take the
fallback.
+ {"failed AND order", null},
+ {"failed OR connection", null},
+ {"order AND NOT failed", null},
+ {"\"place order\"", null},
+ {"\"failed to place order\"", null},
+ // Prefix, wildcard, fuzzy and regexp.
+ {"conn*", null},
+ {"c?che", null},
+ {"connection~1", null},
+ {"/conn.*/", null},
+ // Option-driven parsing must agree too.
+ {"failed order connection", "parser=MATCH,minimumShouldMatch=2"},
+ {"*tion", "allowLeadingWildcard=true"}
+ };
+ }
+
+ @Test(dataProvider = "queries")
+ public void testCountMatchesMaterializedDocIds(String query, String options)
+ throws IOException {
+ try (LuceneTextIndexReader reader = new LuceneTextIndexReader(COLUMN,
INDEX_DIR, DOCS.length, new HashMap<>())) {
Review Comment:
Nit: `new HashMap<>()` (here and in the other three tests) can be `Map.of()`
per the repo convention — the value flows only to
`TextIndexConfigBuilder.withProperties()`, which never mutates it.
--
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]
