You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "deepthi912 (via GitHub)" <gi...@apache.org> on 2025/09/24 06:00:30 UTC
[PR] Configure switch for DICTIONARY scan vs RAW scan in REGEX LIKE evaluation [pinot]
deepthi912 opened a new pull request, #16879:
URL: https://github.com/apache/pinot/pull/16879
**Context:** Currently REGEXP_LIKE without FST/IFST index cases where the number of docs scanned exceed far beyond 10K, and the cardinality percent still remains low even if the cardinality of the dictionary exceeds 10k. So having a % configurable parameter for the user to switch between RAW scan and DICTIONARY based scan is necessary. This PR introduces this switch in 2 ways:
The existing check for dictionary length works as is, if the dictionary length< 10000 it uses dictionary scan. For other cases, the switch was made based on % of docs in a segment both committed and consuming.
Broker Configs:
- Check the threshold limit on the dict usage from `regexpDictCardinalityThreshold`. By default this is set to 10%.
**Example:** select count(*) from mytable where REGEXP_LIKE(NewAddedSVJSONDimension, '.*') OPTION(regexpDictCardinalityThreshold=50000)
`pinot.broker.regexp.dict.cardinality.threshold` - If one wants to set a threshold globally on broker level.
**Order of Priority:**
Query options > Broker Configs
--
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] Configure switch for DICTIONARY scan vs RAW scan in REGEX LIKE evaluation [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #16879:
URL: https://github.com/apache/pinot/pull/16879#discussion_r2374583692
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/RegexpLikePredicateEvaluatorFactory.java:
##########
@@ -36,21 +39,33 @@ public class RegexpLikePredicateEvaluatorFactory {
private RegexpLikePredicateEvaluatorFactory() {
}
- /// When the cardinality of the dictionary is less than this threshold, scan the dictionary to get the matching ids.
- public static final int DICTIONARY_CARDINALITY_THRESHOLD_FOR_SCAN = 10000;
+ /// Default threshold when the cardinality of the dictionary is less than this threshold,
+ // scan the dictionary to get the matching ids.
+ public static final int DEFAULT_DICTIONARY_CARDINALITY_THRESHOLD_FOR_SCAN = 10000;
/**
- * Create a new instance of dictionary based REGEXP_LIKE predicate evaluator.
+ * Create a new instance of dictionary based REGEXP_LIKE predicate evaluator with configurable threshold.
*
* @param regexpLikePredicate REGEXP_LIKE predicate to evaluate
- * @param dictionary Dictionary for the column
- * @param dataType Data type for the column
+ * @param dictionary Dictionary for the column
+ * @param dataType Data type for the column
+ * @param numDocs Number of documents in the segment
+ * @param queryContext Query context containing query options (can be null)
* @return Dictionary based REGEXP_LIKE predicate evaluator
*/
public static BaseDictionaryBasedPredicateEvaluator newDictionaryBasedEvaluator(
- RegexpLikePredicate regexpLikePredicate, Dictionary dictionary, DataType dataType) {
+ RegexpLikePredicate regexpLikePredicate, Dictionary dictionary, DataType dataType, int numDocs,
+ QueryContext queryContext) {
Preconditions.checkArgument(dataType.getStoredType() == DataType.STRING, "Unsupported data type: " + dataType);
- if (dictionary.length() < DICTIONARY_CARDINALITY_THRESHOLD_FOR_SCAN) {
+
+ // Get threshold from query options or use default
+ double threshold = Broker.DEFAULT_REGEXP_LIKE_ADAPTIVE_THRESHOLD;
+ if (queryContext != null && queryContext.getQueryOptions() != null) {
+ threshold = QueryOptionsUtils.getRegexpLikeAdaptiveThreshold(queryContext.getQueryOptions(),
+ Broker.DEFAULT_REGEXP_LIKE_ADAPTIVE_THRESHOLD);
+ }
+ if (dictionary.length() < DEFAULT_DICTIONARY_CARDINALITY_THRESHOLD_FOR_SCAN
+ || (double) dictionary.length() / numDocs < threshold) {
Review Comment:
nit: I think it is worth to move this to its own function to increase readability
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/RegexpLikePredicateEvaluatorFactory.java:
##########
@@ -36,21 +39,33 @@ public class RegexpLikePredicateEvaluatorFactory {
private RegexpLikePredicateEvaluatorFactory() {
}
- /// When the cardinality of the dictionary is less than this threshold, scan the dictionary to get the matching ids.
- public static final int DICTIONARY_CARDINALITY_THRESHOLD_FOR_SCAN = 10000;
+ /// Default threshold when the cardinality of the dictionary is less than this threshold,
+ // scan the dictionary to get the matching ids.
+ public static final int DEFAULT_DICTIONARY_CARDINALITY_THRESHOLD_FOR_SCAN = 10000;
/**
- * Create a new instance of dictionary based REGEXP_LIKE predicate evaluator.
+ * Create a new instance of dictionary based REGEXP_LIKE predicate evaluator with configurable threshold.
*
* @param regexpLikePredicate REGEXP_LIKE predicate to evaluate
- * @param dictionary Dictionary for the column
- * @param dataType Data type for the column
+ * @param dictionary Dictionary for the column
+ * @param dataType Data type for the column
+ * @param numDocs Number of documents in the segment
+ * @param queryContext Query context containing query options (can be null)
* @return Dictionary based REGEXP_LIKE predicate evaluator
*/
public static BaseDictionaryBasedPredicateEvaluator newDictionaryBasedEvaluator(
- RegexpLikePredicate regexpLikePredicate, Dictionary dictionary, DataType dataType) {
+ RegexpLikePredicate regexpLikePredicate, Dictionary dictionary, DataType dataType, int numDocs,
+ QueryContext queryContext) {
Review Comment:
Can you create another static factory method with the same signature as the previous one? Probably nobody outside this repo uses this method, but it is a good practice not to break binary 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] Configure switch for DICTIONARY scan vs RAW scan in REGEX LIKE evaluation [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16879:
URL: https://github.com/apache/pinot/pull/16879#issuecomment-3336106633
Can we revert this one? The `numDocs` used in the PR is total docs in segment instead of docs being scanned, which is not a good indicator for scanning dictionary. I already discussed with @deepthi912 about the idea of caching matched dict ids
--
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] Configure switch for DICTIONARY scan vs RAW scan in REGEX LIKE evaluation [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16879:
URL: https://github.com/apache/pinot/pull/16879#issuecomment-3326863958
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/16879?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `50.00000%` with `13 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 56.33%. Comparing base ([`4ba63e2`](https://app.codecov.io/gh/apache/pinot/commit/4ba63e2b8d1877cd9476ac68aebba2bcbefbbbe9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`58fc677`](https://app.codecov.io/gh/apache/pinot/commit/58fc677c0b0ade23b903ed5dca109312bcaa199d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/16879?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...e/pinot/common/utils/config/QueryOptionsUtils.java](https://app.codecov.io/gh/apache/pinot/pull/16879?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Futils%2Fconfig%2FQueryOptionsUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvY29uZmlnL1F1ZXJ5T3B0aW9uc1V0aWxzLmphdmE=) | 0.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16879?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...predicate/RegexpLikePredicateEvaluatorFactory.java](https://app.codecov.io/gh/apache/pinot/pull/16879?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ffilter%2Fpredicate%2FRegexpLikePredicateEvaluatorFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvcHJlZGljYXRlL1JlZ2V4cExpa2VQcmVkaWNhdGVFdmFsdWF0b3JGYWN0b3J5LmphdmE=) | 20.00% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16879?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 (4ba63e2) and HEAD (58fc677). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (4ba63e2) | HEAD (58fc677) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|10|1|
>|java-11|5|0|
>|unittests2|2|0|
>|integration|6|0|
>|integration1|2|0|
>|custom-integration1|2|0|
>|integration2|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #16879 +/- ##
============================================
- Coverage 63.49% 56.33% -7.16%
+ Complexity 1410 700 -710
============================================
Files 3067 2402 -665
Lines 180097 133549 -46548
Branches 27555 21240 -6315
============================================
- Hits 114355 75237 -39118
+ Misses 56943 52036 -4907
+ Partials 8799 6276 -2523
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/16879/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/16879/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/16879/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/16879/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/16879/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/16879/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/16879/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.33% <50.00%> (-7.12%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/16879/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.33% <50.00%> (-7.16%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/16879/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.33% <50.00%> (-7.16%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/16879/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.33% <50.00%> (-0.05%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/16879/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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/16879?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] Configure switch for DICTIONARY scan vs RAW scan in REGEX LIKE evaluation [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz merged PR #16879:
URL: https://github.com/apache/pinot/pull/16879
--
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