You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "raghavyadav01 (via GitHub)" <gi...@apache.org> on 2025/06/18 22:24:19 UTC

[PR] Allow Users to specify text search options in text search query [pinot]

raghavyadav01 opened a new pull request, #16147:
URL: https://github.com/apache/pinot/pull/16147

   Allow Users to specify text search options in text search query
   
   ### ๐Ÿ“‹ Summary
   This PR introduces a new feature that allows users to specify dynamic query options directly in text search queries using the `__OPTIONS` syntax. This provides runtime flexibility for Lucene query parsing without requiring table configuration changes.
   
   ### โœจ Features
   - **Dynamic Query Options**: Users can now specify Lucene query parser options directly in search strings
   - **Multiple Parser Support**: Supports CLASSIC, STANDARD, and COMPLEX Lucene query parsers
   - **Runtime Configuration**: No need to modify table configs or restart services
   - **Backward Compatibility**: Existing queries continue to work without changes
   
   ### ๐Ÿ”ง Implementation Details
   - Added `__OPTIONS(parser=CLASSIC, allowLeadingWildcard=true, DefaultOperator=AND)` syntax support
   - Implemented dynamic parser instantiation using reflection
   - Added comprehensive option parsing and validation
   - Enhanced `LuceneTextIndexReader` to handle runtime query options
   - Added extensive test coverage for various option combinations
   
   ### Usage Examples
   ```sql
   -- Use CLASSIC parser with leading wildcard support
   SELECT * FROM table WHERE TEXT_MATCH(column, '*search* __OPTIONS(parser=CLASSIC, allowLeadingWildcard=true)')
   
   -- Use STANDARD parser with AND operator
   SELECT * FROM table WHERE TEXT_MATCH(column, 'term1 term2 __OPTIONS(parser=STANDARD, DefaultOperator=AND)')
   
   -- Use COMPLEX parser for complex phrase queries
   SELECT * FROM table WHERE TEXT_MATCH(column, 'complex query __OPTIONS(parser=COMPLEX)')
   ```
   
   ### ๐Ÿงช Testing
   - โœ… Added comprehensive unit tests for option parsing
   - โœ… Added integration tests for different parser types
   - โœ… Added tests for option merging and validation
   - โœ… Added tests for backward compatibility
   - โœ… All existing tests pass
   
   ### ๐Ÿ”„ Backward Compatibility
   - โœ… All existing text search queries continue to work
   - โœ… No changes required to existing table configurations
   - โœ… Default behavior remains unchanged
   


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options as TEXT_MATCH params in text search query [pinot]

Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on PR #16147:
URL: https://github.com/apache/pinot/pull/16147#issuecomment-3001621563

   >>Should we also add this handling to RealtimeLuceneTextIndex?
   Yes, fixed 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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options as TEXT_MATCH params in text search query [pinot]

Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on code in PR #16147:
URL: https://github.com/apache/pinot/pull/16147#discussion_r2162721206


##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtilsTest.java:
##########
@@ -97,4 +97,49 @@ public void testQueryIsNotRewritten() {
     RegexpQuery regexpQuery = new RegexpQuery(new Term("field", "\\d+"));
     Assert.assertEquals(regexpQuery, LuceneTextIndexUtils.convertToMultiTermSpanQuery(regexpQuery));
   }
+
+  @Test
+  public void testParseOptionsString() {
+    // Test null input
+    Map<String, String> result = LuceneTextIndexUtils.parseOptionsString(null);
+    Assert.assertTrue(result.isEmpty());
+
+    // Test empty string
+    result = LuceneTextIndexUtils.parseOptionsString("");
+    Assert.assertTrue(result.isEmpty());
+
+    // Test whitespace-only string
+    result = LuceneTextIndexUtils.parseOptionsString("   ");
+    Assert.assertTrue(result.isEmpty());
+
+    // Test single option
+    result = LuceneTextIndexUtils.parseOptionsString("parser=CLASSIC");
+    Assert.assertEquals(1, result.size());
+    Assert.assertEquals("CLASSIC", result.get("parser"));
+
+    // Test multiple options
+    result = LuceneTextIndexUtils.parseOptionsString("parser=CLASSIC,allowLeadingWildcard=true,DefaultOperator=AND");

Review Comment:
   DefaultOperator, allowLeadingWildcard all these are methods for QueryParser and when they are provided in params we use reflection to call setter methods and set them. Made them consistent in the test. 



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtils.java:
##########
@@ -72,4 +78,148 @@ public static Query convertToMultiTermSpanQuery(Query query) {
     LOGGER.debug("The phrase query {} is re-written as {}", query, spanNearQuery);
     return spanNearQuery;
   }
+
+  /**
+   * Creates a configured Lucene query with the specified options.
+   * This method can be used by both single-column and multi-column text index readers.
+   *
+   * @param actualQuery The cleaned query string without __OPTIONS
+   * @param options The parsed options map
+   * @param column The column name for the search
+   * @param analyzer The Lucene analyzer to use
+   * @return The parsed Lucene Query object
+   * @throws RuntimeException if parser creation or query parsing fails
+   */
+  public static Query createQueryParserWithOptions(String actualQuery, Map<String, String> options, String column,
+      org.apache.lucene.analysis.Analyzer analyzer) {
+    // Get parser type from options
+    String parserType = options.getOrDefault("parser", "CLASSIC");
+    String parserClassName;
+
+    switch (parserType.toUpperCase()) {
+      case "STANDARD":
+        parserClassName = "org.apache.lucene.queryparser.flexible.standard.StandardQueryParser";
+        break;
+      case "COMPLEX":
+        parserClassName = "org.apache.lucene.queryparser.complexPhrase.ComplexPhraseQueryParser";
+        break;
+      default:
+        parserClassName = "org.apache.lucene.queryparser.classic.QueryParser";
+        break;
+    }
+
+    // Create parser instance and apply options
+    try {
+      Class<?> parserClass = Class.forName(parserClassName);
+      Object parser;
+
+      if (parserClassName.equals("org.apache.lucene.queryparser.flexible.standard.StandardQueryParser")) {
+        java.lang.reflect.Constructor<?> constructor = parserClass.getConstructor();
+        parser = constructor.newInstance();
+        try {
+          java.lang.reflect.Method setAnalyzerMethod =
+              parserClass.getMethod("setAnalyzer", org.apache.lucene.analysis.Analyzer.class);
+          setAnalyzerMethod.invoke(parser, analyzer);
+        } catch (Exception e) {
+          LOGGER.warn("Failed to set analyzer on StandardQueryParser: {}", e.getMessage());
+        }
+      } else {
+        // CLASSIC and COMPLEX parsers use the standard (String, Analyzer) constructor
+        java.lang.reflect.Constructor<?> constructor =
+            parserClass.getConstructor(String.class, org.apache.lucene.analysis.Analyzer.class);
+        parser = constructor.newInstance(column, analyzer);
+      }
+
+      // Dynamically apply options using reflection
+      Class<?> clazz = parser.getClass();
+      for (Map.Entry<String, String> entry : options.entrySet()) {
+        String key = entry.getKey();
+        if (key.equals("parser")) {
+          continue; // Skip parser option as it's only used for initialization
+        }
+        String value = entry.getValue();
+        String setterName = "set" + Character.toUpperCase(key.charAt(0)) + key.substring(1);
+
+        boolean found = false;
+        for (java.lang.reflect.Method method : clazz.getMethods()) {
+          if (method.getName().equalsIgnoreCase(setterName) && method.getParameterCount() == 1) {
+            try {
+              Class<?> paramType = method.getParameterTypes()[0];
+              Object paramValue;
+
+              if (paramType == boolean.class || paramType == Boolean.class) {
+                paramValue = Boolean.valueOf(value);
+              } else if (paramType == int.class || paramType == Integer.class) {
+                paramValue = Integer.valueOf(value);
+              } else if (paramType == float.class || paramType == Float.class) {
+                paramValue = Float.valueOf(value);
+              } else if (paramType == double.class || paramType == Double.class) {
+                paramValue = Double.valueOf(value);
+              } else if (paramType.isEnum()) {
+                paramValue = java.lang.Enum.valueOf((Class<java.lang.Enum>) paramType, value);
+              } else {
+                paramValue = value;
+              }
+
+              method.invoke(parser, paramValue);
+              found = true;
+              break;
+            } catch (Exception e) {
+              LOGGER.warn("Failed to apply option {}={}: {}", key, value, e.getMessage());
+            }
+          }
+        }
+
+        if (!found) {
+          LOGGER.warn("Failed to apply option: {}={} (setter not found)", key, value);
+        }
+      }
+
+      // Parse the query using the configured parser
+      Query query;
+      if (parser.getClass().getName().equals("org.apache.lucene.queryparser.flexible.standard.StandardQueryParser")) {
+        // StandardQueryParser uses parse(String, String) where second parameter is default field
+        java.lang.reflect.Method parseMethod = parser.getClass().getMethod("parse", String.class, String.class);
+        query = (Query) parseMethod.invoke(parser, actualQuery, column);
+      } else {
+        // Other parsers use parse(String)
+        java.lang.reflect.Method parseMethod = parser.getClass().getMethod("parse", String.class);
+        query = (Query) parseMethod.invoke(parser, actualQuery);
+      }
+
+      return query;
+    } catch (Exception e) {
+      String msg = "Failed to create or parse query: " + e.getMessage();
+      throw new RuntimeException(msg, e);
+    }
+  }
+
+  /**
+   * Parses options from a separate options string parameter.
+   * The options should be in the format: "key1=value1,key2=value2,key3=value3"
+   *
+   * @param optionsString The options string in key=value format
+   * @return A Map containing the parsed options, or empty map if optionsString is null/empty
+   */
+  public static Map<String, String> parseOptionsString(String optionsString) {

Review Comment:
   Fixed. 



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options as TEXT_MATCH params in text search query [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #16147:
URL: https://github.com/apache/pinot/pull/16147#discussion_r2164756890


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeLuceneTextIndex.java:
##########
@@ -133,6 +135,64 @@ public ImmutableRoaringBitmap getDictIds(String searchQuery) {
 
   @Override
   public MutableRoaringBitmap getDocIds(String searchQuery) {
+    return getDocIdsWithoutOptions(searchQuery);
+  }
+
+  @Override
+  public MutableRoaringBitmap getDocIds(String searchQuery, @Nullable String optionsString) {
+    if (optionsString != null && !optionsString.trim().isEmpty()) {
+      LuceneTextIndexUtils.LuceneTextIndexOptions options = LuceneTextIndexUtils.createOptions(optionsString);
+      Map<String, String> optionsMap = options.getOptions();
+      if (!optionsMap.isEmpty()) {
+        return getDocIdsWithOptions(searchQuery, options);
+      }
+    }
+    return getDocIdsWithoutOptions(searchQuery);
+  }
+
+  private MutableRoaringBitmap getDocIdsWithOptions(String actualQuery,
+      LuceneTextIndexUtils.LuceneTextIndexOptions options) {
+    MutableRoaringBitmap docIDs = new MutableRoaringBitmap();
+    RealtimeLuceneDocIdCollector docIDCollector = new RealtimeLuceneDocIdCollector(docIDs);
+    // A thread interrupt during indexSearcher.search() can break the underlying FSDirectory used by the IndexWriter
+    // which the SearcherManager is created with. To ensure the index is never corrupted the search is executed
+    // in a child thread and the interrupt is handled in the current thread by canceling the search gracefully.
+    // See https://github.com/apache/lucene/issues/3315 and https://github.com/apache/lucene/issues/9309
+    Callable<MutableRoaringBitmap> searchCallable = () -> {
+      IndexSearcher indexSearcher = null;
+      try {
+        Query query = LuceneTextIndexUtils.createQueryParserWithOptions(actualQuery, options, _column, _analyzer);
+        indexSearcher = _searcherManager.acquire();
+        indexSearcher.search(query, docIDCollector);
+        return getPinotDocIds(indexSearcher, docIDs);
+      } finally {
+        try {
+          if (indexSearcher != null) {
+            _searcherManager.release(indexSearcher);
+          }
+        } catch (Exception e) {
+          LOGGER.error(
+              "Failed while releasing the searcher manager for realtime text index for column {}, exception {}",
+              _column, e.getMessage());
+        }
+      }
+    };
+    Future<MutableRoaringBitmap> searchFuture = SEARCHER_POOL.getExecutorService().submit(searchCallable);
+    try {
+      return searchFuture.get();
+    } catch (InterruptedException e) {
+      docIDCollector.markShouldCancel();
+      LOGGER.warn("TEXT_MATCH query interrupted while querying the consuming segment {}, column {}, search query {}",

Review Comment:
   Same here, and other index readers.



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/MultiColumnRealtimeLuceneTextIndex.java:
##########
@@ -149,8 +150,66 @@ public MutableRoaringBitmap getDocIds(String searchQuery) {
     throw new UnsupportedOperationException("Multi-column text index requires column name to query!");
   }
 
+  @Override
+  public MutableRoaringBitmap getDocIds(String column, String searchQuery, @Nullable String optionsString) {

Review Comment:
   The same logic is repeated in 4 places. Let's add a TODO to add a common base class in the future



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/MultiColumnRealtimeLuceneTextIndex.java:
##########
@@ -149,8 +150,66 @@ public MutableRoaringBitmap getDocIds(String searchQuery) {
     throw new UnsupportedOperationException("Multi-column text index requires column name to query!");
   }
 
+  @Override
+  public MutableRoaringBitmap getDocIds(String column, String searchQuery, @Nullable String optionsString) {
+    if (optionsString != null && !optionsString.trim().isEmpty()) {
+      LuceneTextIndexUtils.LuceneTextIndexOptions options = LuceneTextIndexUtils.createOptions(optionsString);
+      Map<String, String> optionsMap = options.getOptions();
+      if (!optionsMap.isEmpty()) {
+        return getDocIdsWithOptions(column, searchQuery, options);
+      }
+    }
+    return getDocIdsWithoutOptions(column, searchQuery);
+  }
+
   @Override
   public MutableRoaringBitmap getDocIds(String column, String searchQuery) {
+    return getDocIdsWithoutOptions(column, searchQuery);
+  }
+
+  private MutableRoaringBitmap getDocIdsWithOptions(String column, String actualQuery,
+      LuceneTextIndexUtils.LuceneTextIndexOptions options) {
+    MutableRoaringBitmap docIDs = new MutableRoaringBitmap();
+    RealtimeLuceneDocIdCollector docIDCollector = new RealtimeLuceneDocIdCollector(docIDs);
+    // A thread interrupt during indexSearcher.search() can break the underlying FSDirectory used by the IndexWriter
+    // which the SearcherManager is created with. To ensure the index is never corrupted the search is executed
+    // in a child thread and the interrupt is handled in the current thread by canceling the search gracefully.
+    // See https://github.com/apache/lucene/issues/3315 and https://github.com/apache/lucene/issues/9309
+    Callable<MutableRoaringBitmap> searchCallable = () -> {
+      IndexSearcher indexSearcher = null;
+      try {
+        Query query = LuceneTextIndexUtils.createQueryParserWithOptions(actualQuery, options, column, _analyzer);
+        indexSearcher = _searcherManager.acquire();
+        indexSearcher.search(query, docIDCollector);
+        return getPinotDocIds(indexSearcher, docIDs);
+      } finally {
+        try {
+          if (indexSearcher != null) {
+            _searcherManager.release(indexSearcher);
+          }
+        } catch (Exception e) {
+          LOGGER.error(
+              "Failed while releasing the searcher manager for realtime text index for columns {}, exception {}",
+              _columns, e.getMessage());
+        }
+      }
+    };
+    Future<MutableRoaringBitmap> searchFuture = SEARCHER_POOL.getExecutorService().submit(searchCallable);
+    try {
+      return searchFuture.get();
+    } catch (InterruptedException e) {
+      docIDCollector.markShouldCancel();
+      LOGGER.warn("TEXT_MATCH query interrupted while querying the consuming segment {}, columns {}, search query {}",
+          _segmentName, _columns, actualQuery);
+      throw new RuntimeException("TEXT_MATCH query interrupted while querying the consuming segment");
+    } catch (Exception e) {
+      LOGGER.error("Failed while searching the realtime text index for segment {}, columns {}, search query {},"

Review Comment:
   Same here. Add the context into the `RuntimeException` instead of logging error



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/MultiColumnRealtimeLuceneTextIndex.java:
##########
@@ -149,8 +150,66 @@ public MutableRoaringBitmap getDocIds(String searchQuery) {
     throw new UnsupportedOperationException("Multi-column text index requires column name to query!");
   }
 
+  @Override
+  public MutableRoaringBitmap getDocIds(String column, String searchQuery, @Nullable String optionsString) {
+    if (optionsString != null && !optionsString.trim().isEmpty()) {
+      LuceneTextIndexUtils.LuceneTextIndexOptions options = LuceneTextIndexUtils.createOptions(optionsString);
+      Map<String, String> optionsMap = options.getOptions();
+      if (!optionsMap.isEmpty()) {
+        return getDocIdsWithOptions(column, searchQuery, options);
+      }
+    }
+    return getDocIdsWithoutOptions(column, searchQuery);
+  }
+
   @Override
   public MutableRoaringBitmap getDocIds(String column, String searchQuery) {
+    return getDocIdsWithoutOptions(column, searchQuery);
+  }
+
+  private MutableRoaringBitmap getDocIdsWithOptions(String column, String actualQuery,
+      LuceneTextIndexUtils.LuceneTextIndexOptions options) {
+    MutableRoaringBitmap docIDs = new MutableRoaringBitmap();
+    RealtimeLuceneDocIdCollector docIDCollector = new RealtimeLuceneDocIdCollector(docIDs);
+    // A thread interrupt during indexSearcher.search() can break the underlying FSDirectory used by the IndexWriter
+    // which the SearcherManager is created with. To ensure the index is never corrupted the search is executed
+    // in a child thread and the interrupt is handled in the current thread by canceling the search gracefully.
+    // See https://github.com/apache/lucene/issues/3315 and https://github.com/apache/lucene/issues/9309
+    Callable<MutableRoaringBitmap> searchCallable = () -> {
+      IndexSearcher indexSearcher = null;
+      try {
+        Query query = LuceneTextIndexUtils.createQueryParserWithOptions(actualQuery, options, column, _analyzer);
+        indexSearcher = _searcherManager.acquire();
+        indexSearcher.search(query, docIDCollector);
+        return getPinotDocIds(indexSearcher, docIDs);
+      } finally {
+        try {
+          if (indexSearcher != null) {
+            _searcherManager.release(indexSearcher);
+          }
+        } catch (Exception e) {
+          LOGGER.error(
+              "Failed while releasing the searcher manager for realtime text index for columns {}, exception {}",
+              _columns, e.getMessage());
+        }
+      }
+    };
+    Future<MutableRoaringBitmap> searchFuture = SEARCHER_POOL.getExecutorService().submit(searchCallable);
+    try {
+      return searchFuture.get();
+    } catch (InterruptedException e) {
+      docIDCollector.markShouldCancel();
+      LOGGER.warn("TEXT_MATCH query interrupted while querying the consuming segment {}, columns {}, search query {}",

Review Comment:
   When we throw out the exception, no need to log warning/error. We usually log the exception when catching/handling 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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options in text search query [pinot]

Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on PR #16147:
URL: https://github.com/apache/pinot/pull/16147#issuecomment-2992286786

   > @raghavyadav01 : can you add options as a third argument instead?
   > 
   > The current approach is slightly unusual and is particularly susceptible to injection attacks (e.g. a hacker using a search bar in an app might attempt to override the options)
   
   Thanks @ankitsultana . 
   
   Security is not compromised because the system only accepts a fixed list of valid parser options (like CLASSIC, STANDARD, COMPLEX). If someone tries to inject invalid or malicious options, the system will simply ignore them. Users can only use the predefined, safe options that we've already tested and approved.
   
   We considered adding a third parameter, but it would require more extensive changes across the codebase and touch more surface area. Since the current text search behavior is inconsistent with Lucene (as some users expect), this fix bridges that gap while minimizing changes/instability and maintaining backward compatibility.


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options in text search query [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16147:
URL: https://github.com/apache/pinot/pull/16147#issuecomment-2986004865

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/16147?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `20.58824%` with `81 lines` in your changes missing coverage. Please review.
   > Project coverage is 33.31%. Comparing base [(`1a476de`)](https://app.codecov.io/gh/apache/pinot/commit/1a476dea72ebf1c15e14ded28ae60c385f71b2c3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`b55623a`)](https://app.codecov.io/gh/apache/pinot/commit/b55623aa568ccb710b6fe6426d9a3c5ffe798b1f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 297 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/16147?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...ment/index/readers/text/LuceneTextIndexReader.java](https://app.codecov.io/gh/apache/pinot/pull/16147?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Findex%2Freaders%2Ftext%2FLuceneTextIndexReader.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvdGV4dC9MdWNlbmVUZXh0SW5kZXhSZWFkZXIuamF2YQ==) | 3.61% | [79 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16147?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...inot/segment/local/utils/LuceneTextIndexUtils.java](https://app.codecov.io/gh/apache/pinot/pull/16147?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Futils%2FLuceneTextIndexUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9MdWNlbmVUZXh0SW5kZXhVdGlscy5qYXZh) | 94.73% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16147?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   > :exclamation:  There is a different number of reports uploaded between BASE (1a476de) and HEAD (b55623a). Click for more details.
   > 
   > <details><summary>HEAD has 68 uploads less than BASE</summary>
   >
   >| Flag | BASE (1a476de) | HEAD (b55623a) |
   >|------|------|------|
   >|java-21|10|1|
   >|unittests|6|1|
   >|unittests1|3|0|
   >|skip-bytebuffers-true|4|0|
   >|temurin|15|1|
   >|unittests2|3|1|
   >|skip-bytebuffers-false|8|0|
   >|java-11|5|0|
   >|integration|9|0|
   >|integration2|3|0|
   >|integration1|3|0|
   >|custom-integration1|3|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #16147       +/-   ##
   =============================================
   - Coverage     62.90%   33.31%   -29.59%     
   + Complexity     1386      659      -727     
   =============================================
     Files          2867     2951       +84     
     Lines        163354   169852     +6498     
     Branches      24952    25976     +1024     
   =============================================
   - Hits         102755    56585    -46170     
   - Misses        52847   108721    +55874     
   + Partials       7752     4546     -3206     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage ฮ” | |
   |---|---|---|
   | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration2](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [java-11](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [java-21](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.31% <20.58%> (-29.51%)` | :arrow_down: |
   | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.31% <20.58%> (-29.59%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.31% <20.58%> (-29.59%)` | :arrow_down: |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/16147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.31% <20.58%> (-0.26%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/16147?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options in text search query [pinot]

Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #16147:
URL: https://github.com/apache/pinot/pull/16147#issuecomment-2992231254

   @raghavyadav01 : can we add options as a third argument instead?
   
   The current approach is slightly unusual and is particularly susceptible to injection attacks (e.g. a hacker using a search bar in an app might attempt to override the options)


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options as TEXT_MATCH params in text search query [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #16147:
URL: https://github.com/apache/pinot/pull/16147


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options in text search query [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #16147:
URL: https://github.com/apache/pinot/pull/16147#discussion_r2162298096


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -161,6 +161,15 @@ public ImmutableRoaringBitmap getDictIds(String searchQuery) {
 
   @Override
   public MutableRoaringBitmap getDocIds(String searchQuery) {
+    java.util.Map.Entry<String, java.util.Map<String, String>> result =

Review Comment:
   (minor) Import `java.util.Map`



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -187,6 +196,23 @@ public MutableRoaringBitmap getDocIds(String searchQuery) {
       throw new RuntimeException(msg, e);
     }
   }
+
+  private MutableRoaringBitmap getDocIdsWithOptions(String actualQuery, Map<String, String> options) {
+    Query query = LuceneTextIndexUtils.createQueryParserWithOptions(actualQuery, options, _column, _analyzer);
+
+    MutableRoaringBitmap docIds = new MutableRoaringBitmap();
+    Collector docIDCollector = new LuceneDocIdCollector(docIds, _docIdTranslator);

Review Comment:
   (minor)
   ```suggestion
       Collector docIdCollector = new LuceneDocIdCollector(docIds, _docIdTranslator);
   ```



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/MultiColumnLuceneTextIndexReader.java:
##########
@@ -351,6 +351,33 @@ public MutableRoaringBitmap getDocIds(String searchQuery) {
 
   @Override
   public MutableRoaringBitmap getDocIds(String column, String searchQuery) {
+    java.util.Map.Entry<String, java.util.Map<String, String>> result =

Review Comment:
   Same for this class



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -161,6 +161,15 @@ public ImmutableRoaringBitmap getDictIds(String searchQuery) {
 
   @Override
   public MutableRoaringBitmap getDocIds(String searchQuery) {
+    java.util.Map.Entry<String, java.util.Map<String, String>> result =
+        LuceneTextIndexUtils.parseOptionsFromSearchString(searchQuery);
+    if (result != null) {
+      return getDocIdsWithOptions(result.getKey(), result.getValue());
+    }
+    return getDocIdsWithoutOptions(searchQuery);
+  }
+
+  private MutableRoaringBitmap getDocIdsWithoutOptions(String searchQuery) {
     MutableRoaringBitmap docIds = new MutableRoaringBitmap();
     Collector docIDCollector = new LuceneDocIdCollector(docIds, _docIdTranslator);
     try {

Review Comment:
   Consider moving this logic also into the utils class



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtils.java:
##########
@@ -72,4 +81,202 @@ public static Query convertToMultiTermSpanQuery(Query query) {
     LOGGER.debug("The phrase query {} is re-written as {}", query, spanNearQuery);
     return spanNearQuery;
   }
+
+  /**
+   * Parses search options from a search query string.
+   * The options can be specified anywhere in the query using __OPTIONS(...) syntax.
+   * For example: "term1 __OPTIONS(parser=CLASSIC) AND term2 __OPTIONS(analyzer=STANDARD)"
+   * Options from later occurrences will override earlier ones.
+   *
+   * @param searchQuery The search query string
+   * @return A Map.Entry containing the cleaned search term and options map, or null if no options found
+   */
+  public static Map.Entry<String, Map<String, String>> parseOptionsFromSearchString(String searchQuery) {
+    try {
+      // Early check for __OPTIONS to avoid unnecessary processing
+      if (searchQuery == null || !searchQuery.contains("__OPTIONS")) {
+        return null;
+      }
+
+      // Pattern to match __OPTIONS(...) with word boundaries
+      // (?<!\\\\) - negative lookbehind to avoid escaped __OPTIONS
+      // \\b - word boundary to ensure __OPTIONS is standalone
+      // __OPTIONS\\((.*?)\\) - non-greedy match for content inside parentheses
+      Pattern pattern = Pattern.compile("(?<!\\\\)\\b__OPTIONS\\((.*?)\\)");

Review Comment:
   Is this regexp match expensive? We have run into similar performance issue before on query option. I'd also suggest keeping the options as a separate argument. You can find example of passing parameters as separate argument in some complex aggregate functions, such as `DistinctCountOffHeapAggregationFunction`



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtils.java:
##########
@@ -72,4 +81,202 @@ public static Query convertToMultiTermSpanQuery(Query query) {
     LOGGER.debug("The phrase query {} is re-written as {}", query, spanNearQuery);
     return spanNearQuery;
   }
+
+  /**
+   * Parses search options from a search query string.
+   * The options can be specified anywhere in the query using __OPTIONS(...) syntax.
+   * For example: "term1 __OPTIONS(parser=CLASSIC) AND term2 __OPTIONS(analyzer=STANDARD)"
+   * Options from later occurrences will override earlier ones.
+   *
+   * @param searchQuery The search query string
+   * @return A Map.Entry containing the cleaned search term and options map, or null if no options found
+   */
+  public static Map.Entry<String, Map<String, String>> parseOptionsFromSearchString(String searchQuery) {

Review Comment:
   (minor) Annotate it with `@Nullable`



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options in text search query [pinot]

Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on code in PR #16147:
URL: https://github.com/apache/pinot/pull/16147#discussion_r2162516226


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtils.java:
##########
@@ -72,4 +81,202 @@ public static Query convertToMultiTermSpanQuery(Query query) {
     LOGGER.debug("The phrase query {} is re-written as {}", query, spanNearQuery);
     return spanNearQuery;
   }
+
+  /**
+   * Parses search options from a search query string.
+   * The options can be specified anywhere in the query using __OPTIONS(...) syntax.
+   * For example: "term1 __OPTIONS(parser=CLASSIC) AND term2 __OPTIONS(analyzer=STANDARD)"
+   * Options from later occurrences will override earlier ones.
+   *
+   * @param searchQuery The search query string
+   * @return A Map.Entry containing the cleaned search term and options map, or null if no options found
+   */
+  public static Map.Entry<String, Map<String, String>> parseOptionsFromSearchString(String searchQuery) {
+    try {
+      // Early check for __OPTIONS to avoid unnecessary processing
+      if (searchQuery == null || !searchQuery.contains("__OPTIONS")) {
+        return null;
+      }
+
+      // Pattern to match __OPTIONS(...) with word boundaries
+      // (?<!\\\\) - negative lookbehind to avoid escaped __OPTIONS
+      // \\b - word boundary to ensure __OPTIONS is standalone
+      // __OPTIONS\\((.*?)\\) - non-greedy match for content inside parentheses
+      Pattern pattern = Pattern.compile("(?<!\\\\)\\b__OPTIONS\\((.*?)\\)");

Review Comment:
   @Jackie-Jiang I have made the changes to take third param in TEXT_MATCH to provide additional options/context to text search. 



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options as TEXT_MATCH params in text search query [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #16147:
URL: https://github.com/apache/pinot/pull/16147#discussion_r2164624941


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtils.java:
##########
@@ -33,10 +38,54 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-
+/**
+ * Utility class for Lucene text index operations.
+ * Contains common methods for parsing options and creating query parsers.
+ */
 public class LuceneTextIndexUtils {
   private static final Logger LOGGER = LoggerFactory.getLogger(LuceneTextIndexUtils.class);
 
+  // Parser option key
+  public static final String PARSER_OPTION = "parser";
+
+  // Parser values
+  public static final String PARSER_CLASSIC = "CLASSIC";
+  public static final String PARSER_STANDARD = "STANDARD";
+  public static final String PARSER_COMPLEX = "COMPLEX";
+
+  // Default operator values
+  public static final String DEFAULT_OPERATOR_AND = "AND";
+  public static final String DEFAULT_OPERATOR_OR = "OR";
+
+  // Boolean values
+  public static final String TRUE = "true";
+  public static final String FALSE = "false";
+
+  // Option keys - Lucene QueryParser configuration options
+  // These options are applied to Lucene QueryParser instances via reflection
+  // Documentation:
+  // https://lucene.apache.org/core/9_8_0/queryparser/org/apache/lucene/queryparser/classic/QueryParser.html
+  public static final String DEFAULT_OPERATOR_OPTION = "DefaultOperator";

Review Comment:
   (minor)
   1. Use lower case for the first character
   2. We can put these option keys under a subclass `OptionKey`, and remove `OPTION` from the name



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/MultiColumnRealtimeLuceneTextIndex.java:
##########
@@ -149,8 +150,65 @@ public MutableRoaringBitmap getDocIds(String searchQuery) {
     throw new UnsupportedOperationException("Multi-column text index requires column name to query!");
   }
 
+  @Override
+  public MutableRoaringBitmap getDocIds(String column, String searchQuery, @Nullable String optionsString) {
+    if (optionsString != null && !optionsString.trim().isEmpty()) {
+      LuceneTextIndexUtils.LuceneTextIndexOptions options = LuceneTextIndexUtils.createOptions(optionsString);
+      Map<String, String> optionsMap = options.getOptions();
+      if (!optionsMap.isEmpty()) {
+        return getDocIdsWithOptions(column, searchQuery, optionsMap);

Review Comment:
   Let's pass `LuceneTextIndexUtils.LuceneTextIndexOptions` all the way down to `LuceneTextIndexUtils.createQueryParserWithOptions()`



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Allow Users to specify text search options as TEXT_MATCH params in text search query [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #16147:
URL: https://github.com/apache/pinot/pull/16147#discussion_r2162623794


##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/predicate/TextMatchPredicate.java:
##########
@@ -27,10 +27,18 @@
  */
 public class TextMatchPredicate extends BasePredicate {
   private final String _value;
+  private final String _options;
 
   public TextMatchPredicate(ExpressionContext lhs, String value) {
     super(lhs);
     _value = value;
+    _options = null;
+  }
+
+  public TextMatchPredicate(ExpressionContext lhs, String value, String options) {
+    super(lhs);
+    _value = value;
+    _options = options;
   }

Review Comment:
   ```suggestion
     public TextMatchPredicate(ExpressionContext lhs, String value) {
       this(lhs, value, null);
     }
   
     public TextMatchPredicate(ExpressionContext lhs, String value, @Nullable String options) {
       super(lhs);
       _value = value;
       _options = options;
     }
   ```



##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/TextIndexReader.java:
##########
@@ -31,9 +31,23 @@ public interface TextIndexReader extends IndexReader {
 
   /**
    * Returns the matching document ids for the given search query.
+   * This is the legacy method for backward compatibility with native/FST text index readers.
    */
   MutableRoaringBitmap getDocIds(String searchQuery);
 
+  /**
+   * Returns the matching document ids for the given search query with options string.
+   * This method allows passing options as a string parameter that will be parsed internally.
+   * Lucene-based text index readers should implement this method.
+   * @param searchQuery The search query string
+   * @param optionsString Options string in format "key1=value1,key2=value2", can be null
+   * @return Matching document ids
+   */
+  default MutableRoaringBitmap getDocIds(String searchQuery, String optionsString) {

Review Comment:
   ```suggestion
     default MutableRoaringBitmap getDocIds(String searchQuery, @Nullable String optionsString) {
   ```



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -159,8 +159,25 @@ public ImmutableRoaringBitmap getDictIds(String searchQuery) {
     throw new UnsupportedOperationException("");
   }
 
+  @Deprecated
   @Override
   public MutableRoaringBitmap getDocIds(String searchQuery) {
+    return getDocIds(searchQuery, null);
+  }
+
+  @Override
+  public MutableRoaringBitmap getDocIds(String searchQuery, String optionsString) {

Review Comment:
   Annotate `optionsString` as `@Nullable`



##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TextMatchTransformFunction.java:
##########
@@ -75,6 +76,19 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
     }
 
     _predicate = ((LiteralTransformFunction) predicate).getStringLiteral();
+
+    // Handle optional third parameter for options
+    if (arguments.size() > 2) {
+      TransformFunction options = arguments.get(2);
+      if (!(options instanceof LiteralTransformFunction && options.getResultMetadata().isSingleValue())) {

Review Comment:
   We can also verify the data type to be string



##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/predicate/TextMatchPredicate.java:
##########
@@ -42,6 +50,10 @@ public String getValue() {
     return _value;
   }
 
+  public String getOptions() {

Review Comment:
   Annotate return as `@Nullable`



##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtilsTest.java:
##########
@@ -97,4 +97,49 @@ public void testQueryIsNotRewritten() {
     RegexpQuery regexpQuery = new RegexpQuery(new Term("field", "\\d+"));
     Assert.assertEquals(regexpQuery, LuceneTextIndexUtils.convertToMultiTermSpanQuery(regexpQuery));
   }
+
+  @Test
+  public void testParseOptionsString() {
+    // Test null input
+    Map<String, String> result = LuceneTextIndexUtils.parseOptionsString(null);
+    Assert.assertTrue(result.isEmpty());
+
+    // Test empty string
+    result = LuceneTextIndexUtils.parseOptionsString("");
+    Assert.assertTrue(result.isEmpty());
+
+    // Test whitespace-only string
+    result = LuceneTextIndexUtils.parseOptionsString("   ");
+    Assert.assertTrue(result.isEmpty());
+
+    // Test single option
+    result = LuceneTextIndexUtils.parseOptionsString("parser=CLASSIC");
+    Assert.assertEquals(1, result.size());
+    Assert.assertEquals("CLASSIC", result.get("parser"));
+
+    // Test multiple options
+    result = LuceneTextIndexUtils.parseOptionsString("parser=CLASSIC,allowLeadingWildcard=true,DefaultOperator=AND");

Review Comment:
   Where is `DefaultOperator` being handled? Can we make it also camel case with first letter in lower case for consistency?



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtils.java:
##########
@@ -72,4 +78,148 @@ public static Query convertToMultiTermSpanQuery(Query query) {
     LOGGER.debug("The phrase query {} is re-written as {}", query, spanNearQuery);
     return spanNearQuery;
   }
+
+  /**
+   * Creates a configured Lucene query with the specified options.
+   * This method can be used by both single-column and multi-column text index readers.
+   *
+   * @param actualQuery The cleaned query string without __OPTIONS
+   * @param options The parsed options map
+   * @param column The column name for the search
+   * @param analyzer The Lucene analyzer to use
+   * @return The parsed Lucene Query object
+   * @throws RuntimeException if parser creation or query parsing fails
+   */
+  public static Query createQueryParserWithOptions(String actualQuery, Map<String, String> options, String column,
+      org.apache.lucene.analysis.Analyzer analyzer) {
+    // Get parser type from options
+    String parserType = options.getOrDefault("parser", "CLASSIC");
+    String parserClassName;
+
+    switch (parserType.toUpperCase()) {
+      case "STANDARD":
+        parserClassName = "org.apache.lucene.queryparser.flexible.standard.StandardQueryParser";
+        break;
+      case "COMPLEX":
+        parserClassName = "org.apache.lucene.queryparser.complexPhrase.ComplexPhraseQueryParser";
+        break;
+      default:
+        parserClassName = "org.apache.lucene.queryparser.classic.QueryParser";
+        break;
+    }
+
+    // Create parser instance and apply options
+    try {
+      Class<?> parserClass = Class.forName(parserClassName);
+      Object parser;
+
+      if (parserClassName.equals("org.apache.lucene.queryparser.flexible.standard.StandardQueryParser")) {
+        java.lang.reflect.Constructor<?> constructor = parserClass.getConstructor();
+        parser = constructor.newInstance();
+        try {
+          java.lang.reflect.Method setAnalyzerMethod =
+              parserClass.getMethod("setAnalyzer", org.apache.lucene.analysis.Analyzer.class);
+          setAnalyzerMethod.invoke(parser, analyzer);
+        } catch (Exception e) {
+          LOGGER.warn("Failed to set analyzer on StandardQueryParser: {}", e.getMessage());
+        }
+      } else {
+        // CLASSIC and COMPLEX parsers use the standard (String, Analyzer) constructor
+        java.lang.reflect.Constructor<?> constructor =
+            parserClass.getConstructor(String.class, org.apache.lucene.analysis.Analyzer.class);
+        parser = constructor.newInstance(column, analyzer);
+      }
+
+      // Dynamically apply options using reflection
+      Class<?> clazz = parser.getClass();
+      for (Map.Entry<String, String> entry : options.entrySet()) {
+        String key = entry.getKey();
+        if (key.equals("parser")) {
+          continue; // Skip parser option as it's only used for initialization
+        }
+        String value = entry.getValue();
+        String setterName = "set" + Character.toUpperCase(key.charAt(0)) + key.substring(1);
+
+        boolean found = false;
+        for (java.lang.reflect.Method method : clazz.getMethods()) {
+          if (method.getName().equalsIgnoreCase(setterName) && method.getParameterCount() == 1) {
+            try {
+              Class<?> paramType = method.getParameterTypes()[0];
+              Object paramValue;
+
+              if (paramType == boolean.class || paramType == Boolean.class) {
+                paramValue = Boolean.valueOf(value);
+              } else if (paramType == int.class || paramType == Integer.class) {
+                paramValue = Integer.valueOf(value);
+              } else if (paramType == float.class || paramType == Float.class) {
+                paramValue = Float.valueOf(value);
+              } else if (paramType == double.class || paramType == Double.class) {
+                paramValue = Double.valueOf(value);
+              } else if (paramType.isEnum()) {
+                paramValue = java.lang.Enum.valueOf((Class<java.lang.Enum>) paramType, value);
+              } else {
+                paramValue = value;
+              }
+
+              method.invoke(parser, paramValue);
+              found = true;
+              break;
+            } catch (Exception e) {
+              LOGGER.warn("Failed to apply option {}={}: {}", key, value, e.getMessage());
+            }
+          }
+        }
+
+        if (!found) {
+          LOGGER.warn("Failed to apply option: {}={} (setter not found)", key, value);
+        }
+      }
+
+      // Parse the query using the configured parser
+      Query query;
+      if (parser.getClass().getName().equals("org.apache.lucene.queryparser.flexible.standard.StandardQueryParser")) {
+        // StandardQueryParser uses parse(String, String) where second parameter is default field
+        java.lang.reflect.Method parseMethod = parser.getClass().getMethod("parse", String.class, String.class);
+        query = (Query) parseMethod.invoke(parser, actualQuery, column);
+      } else {
+        // Other parsers use parse(String)
+        java.lang.reflect.Method parseMethod = parser.getClass().getMethod("parse", String.class);
+        query = (Query) parseMethod.invoke(parser, actualQuery);
+      }
+
+      return query;
+    } catch (Exception e) {
+      String msg = "Failed to create or parse query: " + e.getMessage();
+      throw new RuntimeException(msg, e);
+    }
+  }
+
+  /**
+   * Parses options from a separate options string parameter.
+   * The options should be in the format: "key1=value1,key2=value2,key3=value3"
+   *
+   * @param optionsString The options string in key=value format
+   * @return A Map containing the parsed options, or empty map if optionsString is null/empty
+   */
+  public static Map<String, String> parseOptionsString(String optionsString) {

Review Comment:
   We can introduce a class to wrap the parsed options. Similar to `DistinctCountOffHeapAggregationFunction.Parameters`



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/LuceneTextIndexUtils.java:
##########
@@ -72,4 +78,148 @@ public static Query convertToMultiTermSpanQuery(Query query) {
     LOGGER.debug("The phrase query {} is re-written as {}", query, spanNearQuery);
     return spanNearQuery;
   }
+
+  /**
+   * Creates a configured Lucene query with the specified options.
+   * This method can be used by both single-column and multi-column text index readers.
+   *
+   * @param actualQuery The cleaned query string without __OPTIONS
+   * @param options The parsed options map
+   * @param column The column name for the search
+   * @param analyzer The Lucene analyzer to use
+   * @return The parsed Lucene Query object
+   * @throws RuntimeException if parser creation or query parsing fails
+   */
+  public static Query createQueryParserWithOptions(String actualQuery, Map<String, String> options, String column,
+      org.apache.lucene.analysis.Analyzer analyzer) {
+    // Get parser type from options
+    String parserType = options.getOrDefault("parser", "CLASSIC");

Review Comment:
   Make keys and values constant



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org