You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "egalpin (via GitHub)" <gi...@apache.org> on 2024/02/13 22:22:44 UTC
[PR] Adds per-column, query-time index skip option [pinot]
egalpin opened a new pull request, #12414:
URL: https://github.com/apache/pinot/pull/12414
Closes https://github.com/apache/pinot/issues/12355. Needs tests but opening for early feedback.
cc @kishoreg @Jackie-Jiang
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on code in PR #12414:
URL: https://github.com/apache/pinot/pull/12414#discussion_r1498242645
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -145,6 +149,33 @@ public static boolean isSkipScanFilterReorder(Map<String, String> queryOptions)
return "false".equalsIgnoreCase(queryOptions.get(QueryOptionKey.USE_SCAN_REORDER_OPTIMIZATION));
}
+ @Nullable
+ public static Map<String, Set<FieldConfig.IndexType>> getIndexSkipConfig(Map<String, String> queryOptions) {
+ // Example config: indexSkipConfig='col1=inverted,range&col2=inverted'
Review Comment:
should I look for an OSS URL param parser? Not complex code particularly, but also not a unique problem to need to parse url params
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on PR #12414:
URL: https://github.com/apache/pinot/pull/12414#issuecomment-1957810245
tests added, this is ready for review.
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on PR #12414:
URL: https://github.com/apache/pinot/pull/12414#issuecomment-1964823595
@gortiz anything that needs attention at the moment? I want to be sure I've addressed any comments and can't find any unaddressed at the moment.
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #12414:
URL: https://github.com/apache/pinot/pull/12414#issuecomment-1959184631
It LGTM. As said in the linked issue, my main concern is on the language to use. I think URL args is fine, but maybe we would need to have a bigger picture because the language we decide to use here is what we should use in future options with the same type
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz merged PR #12414:
URL: https://github.com/apache/pinot/pull/12414
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #12414:
URL: https://github.com/apache/pinot/pull/12414#discussion_r1499056482
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -145,6 +149,33 @@ public static boolean isSkipScanFilterReorder(Map<String, String> queryOptions)
return "false".equalsIgnoreCase(queryOptions.get(QueryOptionKey.USE_SCAN_REORDER_OPTIMIZATION));
}
+ @Nullable
+ public static Map<String, Set<FieldConfig.IndexType>> getIndexSkipConfig(Map<String, String> queryOptions) {
+ // Example config: indexSkipConfig='col1=inverted,range&col2=inverted'
+ String indexSkipConfigStr = queryOptions.get(QueryOptionKey.INDEX_SKIP_CONFIG);
+ if (indexSkipConfigStr == null) {
+ return null;
+ }
+
+ String[] perColumnIndexSkip = indexSkipConfigStr.split("&");
+ Map<String, Set<FieldConfig.IndexType>> indexSkipConfig = new HashMap<>();
+ for (String columnConf : perColumnIndexSkip) {
+ String[] conf = columnConf.split("=");
+ if (conf.length != 2) {
+ continue;
Review Comment:
Shouldn't we fail in this case?
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on code in PR #12414:
URL: https://github.com/apache/pinot/pull/12414#discussion_r1499580763
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -145,6 +149,33 @@ public static boolean isSkipScanFilterReorder(Map<String, String> queryOptions)
return "false".equalsIgnoreCase(queryOptions.get(QueryOptionKey.USE_SCAN_REORDER_OPTIMIZATION));
}
+ @Nullable
+ public static Map<String, Set<FieldConfig.IndexType>> getIndexSkipConfig(Map<String, String> queryOptions) {
+ // Example config: indexSkipConfig='col1=inverted,range&col2=inverted'
+ String indexSkipConfigStr = queryOptions.get(QueryOptionKey.INDEX_SKIP_CONFIG);
+ if (indexSkipConfigStr == null) {
+ return null;
+ }
+
+ String[] perColumnIndexSkip = indexSkipConfigStr.split("&");
+ Map<String, Set<FieldConfig.IndexType>> indexSkipConfig = new HashMap<>();
+ for (String columnConf : perColumnIndexSkip) {
+ String[] conf = columnConf.split("=");
+ if (conf.length != 2) {
+ continue;
Review Comment:
I think so, ya. I feel like failed parsing is better to end up as a failed query than a successful query without the intended options set.
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on code in PR #12414:
URL: https://github.com/apache/pinot/pull/12414#discussion_r1499609689
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -145,6 +149,33 @@ public static boolean isSkipScanFilterReorder(Map<String, String> queryOptions)
return "false".equalsIgnoreCase(queryOptions.get(QueryOptionKey.USE_SCAN_REORDER_OPTIMIZATION));
}
+ @Nullable
+ public static Map<String, Set<FieldConfig.IndexType>> getIndexSkipConfig(Map<String, String> queryOptions) {
+ // Example config: indexSkipConfig='col1=inverted,range&col2=inverted'
+ String indexSkipConfigStr = queryOptions.get(QueryOptionKey.INDEX_SKIP_CONFIG);
+ if (indexSkipConfigStr == null) {
+ return null;
+ }
+
+ String[] perColumnIndexSkip = indexSkipConfigStr.split("&");
+ Map<String, Set<FieldConfig.IndexType>> indexSkipConfig = new HashMap<>();
+ for (String columnConf : perColumnIndexSkip) {
+ String[] conf = columnConf.split("=");
+ if (conf.length != 2) {
+ continue;
Review Comment:
Actually it looks like there isn't precedent for throwing exceptions from within `InstancePlanMakerImplV2#applyQueryOptions`, so the "easiest" would be a `RuntimeException`
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on PR #12414:
URL: https://github.com/apache/pinot/pull/12414#issuecomment-1969505872
@Jackie-Jiang I'll make a follow-up PR to address these by end of week
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on code in PR #12414:
URL: https://github.com/apache/pinot/pull/12414#discussion_r1499585136
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -145,6 +149,33 @@ public static boolean isSkipScanFilterReorder(Map<String, String> queryOptions)
return "false".equalsIgnoreCase(queryOptions.get(QueryOptionKey.USE_SCAN_REORDER_OPTIMIZATION));
}
+ @Nullable
+ public static Map<String, Set<FieldConfig.IndexType>> getIndexSkipConfig(Map<String, String> queryOptions) {
+ // Example config: indexSkipConfig='col1=inverted,range&col2=inverted'
+ String indexSkipConfigStr = queryOptions.get(QueryOptionKey.INDEX_SKIP_CONFIG);
+ if (indexSkipConfigStr == null) {
+ return null;
+ }
+
+ String[] perColumnIndexSkip = indexSkipConfigStr.split("&");
+ Map<String, Set<FieldConfig.IndexType>> indexSkipConfig = new HashMap<>();
+ for (String columnConf : perColumnIndexSkip) {
+ String[] conf = columnConf.split("=");
+ if (conf.length != 2) {
+ continue;
Review Comment:
@gortiz would a `org.apache.pinot.sql.parsers.parser.ParseException` be a good choice here? I don't know if that exception is reserved for SQL _not including_ query options, though.
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12414:
URL: https://github.com/apache/pinot/pull/12414#issuecomment-1942780785
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `43 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`7c9bf8c`)](https://app.codecov.io/gh/apache/pinot/commit/7c9bf8cf49b0ed78f1de26edffa602bd3f74f548?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.77% compared to head [(`e91f31a`)](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 0.00%.
> Report is 10 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12414?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/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvY29uZmlnL1F1ZXJ5T3B0aW9uc1V0aWxzLmphdmE=) | 0.00% | [15 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...inot/core/operator/filter/FilterOperatorUtils.java](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvRmlsdGVyT3BlcmF0b3JVdGlscy5qYXZh) | 0.00% | [14 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pinot/core/query/request/context/QueryContext.java](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9yZXF1ZXN0L2NvbnRleHQvUXVlcnlDb250ZXh0LmphdmE=) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pache/pinot/segment/spi/datasource/DataSource.java](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2RhdGFzb3VyY2UvRGF0YVNvdXJjZS5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12414?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12414 +/- ##
=============================================
- Coverage 61.77% 0.00% -61.78%
=============================================
Files 2425 2361 -64
Lines 132753 129383 -3370
Branches 20535 20074 -461
=============================================
- Hits 82005 0 -82005
- Misses 44730 129383 +84653
+ Partials 6018 0 -6018
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12414/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/12414/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/12414/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-0.01%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12414/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/12414/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12414/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/12414/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.65%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12414/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.73%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12414/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/12414/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.78%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12414/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12414/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/12414/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/12414?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).
--
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] Adds per-column, query-time index skip option [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12414:
URL: https://github.com/apache/pinot/pull/12414#discussion_r1505096337
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -145,6 +149,34 @@ public static boolean isSkipScanFilterReorder(Map<String, String> queryOptions)
return "false".equalsIgnoreCase(queryOptions.get(QueryOptionKey.USE_SCAN_REORDER_OPTIMIZATION));
}
+ @Nullable
+ public static Map<String, Set<FieldConfig.IndexType>> getIndexSkipConfig(Map<String, String> queryOptions) {
+ // Example config: indexSkipConfig='col1=inverted,range&col2=inverted'
+ String indexSkipConfigStr = queryOptions.get(QueryOptionKey.INDEX_SKIP_CONFIG);
+ if (indexSkipConfigStr == null) {
+ return null;
+ }
+
+ String[] perColumnIndexSkip = indexSkipConfigStr.split("&");
Review Comment:
Suggest using `StringUtils.split(indexSkipConfigStr, '&')` which is more lightweight. The current API will try to do regex match. Same for other split calls
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -361,6 +361,7 @@ public static class QueryOptionKey {
public static final String SERVER_RETURN_FINAL_RESULT = "serverReturnFinalResult";
// Reorder scan based predicates based on cardinality and number of selected values
public static final String AND_SCAN_REORDERING = "AndScanReordering";
+ public static final String INDEX_SKIP_CONFIG = "indexSkipConfig";
Review Comment:
(personal preference) I feel `skipIndexes` is more consistent with other option keys
--
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