You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2023/11/07 00:37:41 UTC
[PR] Support constant filter in QueryContext, and make server able to handle it [pinot]
Jackie-Jiang opened a new pull request, #11956:
URL: https://github.com/apache/pinot/pull/11956
Currently we rely on broker to remove the constant (true/false) filter, and server doesn't support processing constant filter. This PR adds the server side support to handle constant filter which is needed for multi-stage engine as leaf stage
--
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] Support constant filter in QueryContext, and make server able to handle it [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11956:
URL: https://github.com/apache/pinot/pull/11956
--
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] Support constant filter in QueryContext, and make server able to handle it [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #11956:
URL: https://github.com/apache/pinot/pull/11956#discussion_r1385174026
##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/RequestContextUtils.java:
##########
@@ -111,11 +111,9 @@ public static FilterContext getFilter(Expression thriftExpression) {
return getFilter(thriftFunction);
case IDENTIFIER:
// Convert "WHERE a" to "WHERE a = true"
- return new FilterContext(FilterContext.Type.PREDICATE, null,
- new EqPredicate(getExpression(thriftExpression), getStringValue(RequestUtils.getLiteralExpression(true))));
+ return FilterContext.forPredicate(new EqPredicate(getExpression(thriftExpression), "true"));
case LITERAL:
- // TODO: Handle literals.
- throw new IllegalStateException();
+ return FilterContext.forConstant(new LiteralContext(thriftExpression.getLiteral()).getBooleanValue());
Review Comment:
general question: why do we need to repeat ourselves on these utils one for thrift and one for function context?
it looks like the only difference is one is for WHERE and one is for HAVING?
##########
pinot-core/src/test/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverterTest.java:
##########
@@ -658,7 +658,7 @@ public void testFilteredAggregations() {
}
@Test
- void testDeduplicateOrderByExpressions() {
+ public void testDeduplicateOrderByExpressions() {
Review Comment:
LOL
--
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] Support constant filter in QueryContext, and make server able to handle it [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11956:
URL: https://github.com/apache/pinot/pull/11956#discussion_r1385237914
##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/RequestContextUtils.java:
##########
@@ -111,11 +111,9 @@ public static FilterContext getFilter(Expression thriftExpression) {
return getFilter(thriftFunction);
case IDENTIFIER:
// Convert "WHERE a" to "WHERE a = true"
- return new FilterContext(FilterContext.Type.PREDICATE, null,
- new EqPredicate(getExpression(thriftExpression), getStringValue(RequestUtils.getLiteralExpression(true))));
+ return FilterContext.forPredicate(new EqPredicate(getExpression(thriftExpression), "true"));
case LITERAL:
- // TODO: Handle literals.
- throw new IllegalStateException();
+ return FilterContext.forConstant(new LiteralContext(thriftExpression.getLiteral()).getBooleanValue());
Review Comment:
One is used in aggregation FILTER where the expression is already parsed into context format
--
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] Support constant filter in QueryContext, and make server able to handle it [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11956:
URL: https://github.com/apache/pinot/pull/11956#issuecomment-1797120527
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11956](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (e4f07d2) into [master](https://app.codecov.io/gh/apache/pinot/commit/baea4a20dfa0fb56e545078c7fa11e01a478e681?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (baea4a2) will **decrease** coverage by `61.46%`.
> Report is 2 commits behind head on master.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11956 +/- ##
=============================================
- Coverage 61.45% 0.00% -61.46%
+ Complexity 1145 6 -1139
=============================================
Files 2385 2309 -76
Lines 129065 125389 -3676
Branches 19955 19433 -522
=============================================
- Hits 79317 6 -79311
- Misses 44023 125383 +81360
+ Partials 5725 0 -5725
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/11956/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/11956/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/11956/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/11956/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/11956/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/11956/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/11956/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.30%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/11956/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.42%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/11956/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.29%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/11956/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-61.46%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/11956/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/11956/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/11956/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.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...n/DistinctCountThetaSketchAggregationFunction.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50VGhldGFTa2V0Y2hBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `0.00% <0.00%> (-63.57%)` | :arrow_down: |
| [...local/realtime/impl/json/MutableJsonIndexImpl.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2pzb24vTXV0YWJsZUpzb25JbmRleEltcGwuamF2YQ==) | `0.00% <0.00%> (-90.26%)` | :arrow_down: |
| [...t/index/readers/json/ImmutableJsonIndexReader.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvanNvbi9JbW11dGFibGVKc29uSW5kZXhSZWFkZXIuamF2YQ==) | `0.00% <0.00%> (-77.78%)` | :arrow_down: |
| [...ntroller/recommender/rules/impl/FlagQueryRule.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL0ZsYWdRdWVyeVJ1bGUuamF2YQ==) | `0.00% <0.00%> (-95.00%)` | :arrow_down: |
| [...es/impl/NoDictionaryOnHeapDictionaryJointRule.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL05vRGljdGlvbmFyeU9uSGVhcERpY3Rpb25hcnlKb2ludFJ1bGUuamF2YQ==) | `0.00% <0.00%> (-97.00%)` | :arrow_down: |
| [...troller/recommender/rules/impl/RangeIndexRule.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL1JhbmdlSW5kZXhSdWxlLmphdmE=) | `0.00% <0.00%> (-95.35%)` | :arrow_down: |
| [.../pinot/controller/recommender/io/InputManager.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9JbnB1dE1hbmFnZXIuamF2YQ==) | `0.00% <0.00%> (-93.23%)` | :arrow_down: |
| [...roller/recommender/rules/impl/BloomFilterRule.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL0Jsb29tRmlsdGVyUnVsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ecommender/rules/impl/PinotTablePartitionRule.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL1Bpbm90VGFibGVQYXJ0aXRpb25SdWxlLmphdmE=) | `0.00% <0.00%> (-79.82%)` | :arrow_down: |
| [...les/utils/QueryInvertedSortedIndexRecommender.java](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy91dGlscy9RdWVyeUludmVydGVkU29ydGVkSW5kZXhSZWNvbW1lbmRlci5qYXZh) | `0.00% <0.00%> (-82.02%)` | :arrow_down: |
| ... and [4 more](https://app.codecov.io/gh/apache/pinot/pull/11956?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [1976 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11956/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in [Chrome](https://chrome.google.com/webstore/detail/codecov/gedikamndpbemklijjkncpnolildpbgo) or [Firefox](https://addons.mozilla.org/en-US/firefox/addon/codecov/) today!
--
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