xiangfu0 commented on code in PR #18758:
URL: https://github.com/apache/pinot/pull/18758#discussion_r4505402259
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java:
##########
@@ -306,6 +308,26 @@ private BaseFilterOperator
constructPhysicalOperator(FilterContext filter, int n
Preconditions.checkState(textIndexReader != null,
"Cannot apply TEXT_MATCH on column: %s without text index",
column);
+ if (textIndexReader.isBuildOnDictionary()) {
+ // Dictionary-based text index: search returns matching
dictIds, resolved to docIds by the standard
+ // dictionary-based filter operators (inverted index / scan),
exactly like FST/REGEXP_LIKE.
+ TextMatchPredicate textMatchPredicate = (TextMatchPredicate)
predicate;
+ Preconditions.checkState(textMatchPredicate.getOptions() ==
null,
+ "TEXT_MATCH options are not supported for a
dictionary-based text index on column: %s", column);
+ predicateEvaluator =
TextMatchDictIdPredicateEvaluatorFactory.newDictIdBasedEvaluator(
+ textMatchPredicate, textIndexReader,
dataSource.getDictionary());
+ BaseFilterOperator filterOperator =
+ FilterOperatorUtils.getLeafFilterOperator(_queryContext,
predicateEvaluator, dataSource, numDocs);
Review Comment:
Fixed in
[075aa77f18](https://github.com/apache/pinot/commit/075aa77f1824c43b0cee2bfe00b29c3b9891283c).
Dictionary TEXT_MATCH now preserves the existing per-row behavior for stored
null defaults, including NOT and constant-match shortcuts, while other
predicates retain query null handling.
The new DictionaryBasedTextIndexQueriesTest regression reproduced the
mismatch (expected 2 rows, got 1) and now passes for both scan and
inverted-index paths, including selected row IDs and mixed null predicates. All
510 focused tests passed on JDK 25; affected-module formatting, checkstyle,
license checks, and independent review also passed.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/text/TextIndexType.java:
##########
@@ -84,8 +86,14 @@ public TextIndexConfig getDefaultConfig() {
public void validate(FieldIndexConfigs indexConfigs, FieldSpec fieldSpec,
TableConfig tableConfig) {
TextIndexConfig textIndexConfig =
indexConfigs.getConfig(StandardIndexes.text());
if (textIndexConfig.isEnabled()) {
- Preconditions.checkState(fieldSpec.getDataType().getStoredType() ==
FieldSpec.DataType.STRING,
+ Preconditions.checkState(fieldSpec.getDataType().getStoredType() ==
DataType.STRING,
"Cannot create TEXT index on column: %s of stored type other than
STRING", fieldSpec.getName());
+ if (textIndexConfig.isBuildOnDictionary()) {
+ Preconditions.checkState(fieldSpec.isSingleValueField(),
+ "buildOnDictionary TEXT index requires a single-value column: %s",
fieldSpec.getName());
+
Preconditions.checkState(indexConfigs.getConfig(StandardIndexes.dictionary()).isEnabled(),
+ "buildOnDictionary TEXT index requires a dictionary-encoded
column: %s", fieldSpec.getName());
Review Comment:
Added guidance in TextIndexConfig.getBuildOnDictionary() in
[075aa77f18](https://github.com/apache/pinot/commit/075aa77f1824c43b0cee2bfe00b29c3b9891283c):
pair buildOnDictionary with an inverted index for unsorted columns. Sorted
columns can use their sorted index; without a suitable index, TEXT_MATCH scans
the forward index. Affected-module checks and the focused JDK 25 test suite
passed.
--
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]
