raghavyadav01 commented on code in PR #18758:
URL: https://github.com/apache/pinot/pull/18758#discussion_r4505329456
##########
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:
With null handling on, this path drops null rows via
`BaseColumnFilterOperator`, but `TextMatchFilterOperator` passes
`nullHandlingEnabled=false`. So `NOT TEXT_MATCH(...)` could return different
rows depending on how each segment was built. Is that intended? Could we align
the two modes and add a null-handling case to
`DictionaryBasedTextIndexQueriesTest`?
##########
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:
Without an inverted index on the column, dictionary-mode `TEXT_MATCH` falls
back to a full scan, where the per-row index answers from a bitmap. Should we
recommend (or warn about) pairing `buildOnDictionary` with an inverted index?
--
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]
