You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/03/20 05:41:26 UTC
[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #5168:
Refactor DistinctQueriesTest to cover more scenarios
Jackie-Jiang opened a new pull request #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168
- Test all data types (including BYTES)
- Test empty responses
- Test merge of non-empty response and empty response for both server side and broker side
TODO:
- ORDER BY bytesColumn is not properly supported right now
- DistinceTable limit is not properly 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #5168:
Refactor DistinctQueriesTest to cover more scenarios
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168#issuecomment-601551841
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=h1) Report
> Merging [#5168](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1f1baf85fb318caa48b3b207e032946ddd5de11c&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `71.57%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5168 +/- ##
============================================
- Coverage 65.90% 65.87% -0.04%
Complexity 12 12
============================================
Files 1052 1055 +3
Lines 54170 54097 -73
Branches 8078 8047 -31
============================================
- Hits 35702 35636 -66
+ Misses 15819 15815 -4
+ Partials 2649 2646 -3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `76.66% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [.../BrokerResourceOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclJlc291cmNlT25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `55.81% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [.../pinot/broker/broker/helix/HelixBrokerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0hlbGl4QnJva2VyU3RhcnRlci5qYXZh) | `71.97% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...thandler/SingleConnectionBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvU2luZ2xlQ29ubmVjdGlvbkJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `92.68% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...rg/apache/pinot/broker/routing/RoutingManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9Sb3V0aW5nTWFuYWdlci5qYXZh) | `78.84% <ø> (-2.31%)` | `0.00 <0.00> (ø)` | |
| [...ting/instanceselector/InstanceSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3JGYWN0b3J5LmphdmE=) | `71.42% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...er/routing/segmentpruner/SegmentPrunerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1NlZ21lbnRQcnVuZXJGYWN0b3J5LmphdmE=) | `83.33% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...outing/segmentselector/SegmentSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50c2VsZWN0b3IvU2VnbWVudFNlbGVjdG9yRmFjdG9yeS5qYXZh) | `60.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...oker/routing/timeboundary/TimeBoundaryManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy90aW1lYm91bmRhcnkvVGltZUJvdW5kYXJ5TWFuYWdlci5qYXZh) | `87.50% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...mmon/assignment/InstanceAssignmentConfigUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZUFzc2lnbm1lbnRDb25maWdVdGlscy5qYXZh) | `67.50% <ø> (ø)` | `0.00 <0.00> (?)` | |
| ... and [170 more](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=footer). Last update [00fcb1d...65a7042](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull
request #5168: Refactor DistinctQueriesTest to cover more scenarios
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168#discussion_r401189152
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/reduce/DistinctDataTableReducer.java
##########
@@ -108,7 +108,7 @@ public void reduceAndSetResults(String tableName, DataSchema dataSchema,
if (mergedIntermediateResult == null) {
mergedIntermediateResult = intermediateResultToMerge;
} else {
- _aggregationFunction.merge(mergedIntermediateResult, intermediateResultToMerge);
+ mergedIntermediateResult = _aggregationFunction.merge(mergedIntermediateResult, intermediateResultToMerge);
Review comment:
IIUC, this is needed because for the case of empty, it may return a new object as opposed to merging into existing ones?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #5168:
Refactor DistinctQueriesTest to cover more scenarios
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168#issuecomment-601551841
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=h1) Report
> Merging [#5168](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/d289db85b350e562d3c0b123ad64c7a2637db4ac&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5168 +/- ##
============================================
- Coverage 66.22% 66.12% -0.10%
Complexity 12 12
============================================
Files 1051 1051
Lines 54119 54120 +1
Branches 8064 8064
============================================
- Hits 35840 35788 -52
- Misses 15630 15675 +45
- Partials 2649 2657 +8
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...he/pinot/core/query/aggregation/DistinctTable.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9EaXN0aW5jdFRhYmxlLmphdmE=) | `83.52% <100.00%> (+7.33%)` | `0.00 <0.00> (ø)` | |
| [...ot/core/query/reduce/DistinctDataTableReducer.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9yZWR1Y2UvRGlzdGluY3REYXRhVGFibGVSZWR1Y2VyLmphdmE=) | `67.74% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50.00% <0.00%> (-25.00%)` | `0.00% <0.00%> (ø%)` | |
| [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `62.19% <0.00%> (-14.64%)` | `0.00% <0.00%> (ø%)` | |
| [...ore/query/scheduler/resources/ResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvcmVzb3VyY2VzL1Jlc291cmNlTWFuYWdlci5qYXZh) | `85.71% <0.00%> (-10.72%)` | `0.00% <0.00%> (ø%)` | |
| [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `52.43% <0.00%> (-8.54%)` | `0.00% <0.00%> (ø%)` | |
| [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `60.21% <0.00%> (-7.53%)` | `0.00% <0.00%> (ø%)` | |
| [.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `70.73% <0.00%> (-6.10%)` | `0.00% <0.00%> (ø%)` | |
| [...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=) | `39.47% <0.00%> (-5.27%)` | `0.00% <0.00%> (ø%)` | |
| [...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `49.46% <0.00%> (-4.31%)` | `0.00% <0.00%> (ø%)` | |
| ... and [21 more](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=footer). Last update [d289db8...54488d9](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #5168:
Refactor DistinctQueriesTest to cover more scenarios
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168#issuecomment-601551841
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=h1) Report
> Merging [#5168](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/d289db85b350e562d3c0b123ad64c7a2637db4ac&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5168 +/- ##
============================================
- Coverage 66.22% 66.12% -0.10%
Complexity 12 12
============================================
Files 1051 1051
Lines 54119 54120 +1
Branches 8064 8064
============================================
- Hits 35840 35788 -52
- Misses 15630 15675 +45
- Partials 2649 2657 +8
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...he/pinot/core/query/aggregation/DistinctTable.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9EaXN0aW5jdFRhYmxlLmphdmE=) | `83.52% <100.00%> (+7.33%)` | `0.00 <0.00> (ø)` | |
| [...ot/core/query/reduce/DistinctDataTableReducer.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9yZWR1Y2UvRGlzdGluY3REYXRhVGFibGVSZWR1Y2VyLmphdmE=) | `67.74% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50.00% <0.00%> (-25.00%)` | `0.00% <0.00%> (ø%)` | |
| [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `62.19% <0.00%> (-14.64%)` | `0.00% <0.00%> (ø%)` | |
| [...ore/query/scheduler/resources/ResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvcmVzb3VyY2VzL1Jlc291cmNlTWFuYWdlci5qYXZh) | `85.71% <0.00%> (-10.72%)` | `0.00% <0.00%> (ø%)` | |
| [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `52.43% <0.00%> (-8.54%)` | `0.00% <0.00%> (ø%)` | |
| [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `60.21% <0.00%> (-7.53%)` | `0.00% <0.00%> (ø%)` | |
| [.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `70.73% <0.00%> (-6.10%)` | `0.00% <0.00%> (ø%)` | |
| [...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=) | `39.47% <0.00%> (-5.27%)` | `0.00% <0.00%> (ø%)` | |
| [...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `49.46% <0.00%> (-4.31%)` | `0.00% <0.00%> (ø%)` | |
| ... and [21 more](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=footer). Last update [d289db8...54488d9](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #5168: Refactor
DistinctQueriesTest to cover more scenarios
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168#issuecomment-601551841
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=h1) Report
> Merging [#5168](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/d289db85b350e562d3c0b123ad64c7a2637db4ac&el=desc) will **decrease** coverage by `20.95%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5168 +/- ##
=============================================
- Coverage 66.02% 45.07% -20.96%
=============================================
Files 1051 1051
Lines 54119 54120 +1
Branches 8064 8064
=============================================
- Hits 35730 24392 -11338
- Misses 15739 27615 +11876
+ Partials 2650 2113 -537
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...he/pinot/core/query/aggregation/DistinctTable.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9EaXN0aW5jdFRhYmxlLmphdmE=) | `55.29% <0.00%> (-20.90%)` | `0.00 <0.00> (ø)` | |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EaW1lbnNpb25GaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/pinot/spi/data/readers/FileFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...va/org/apache/pinot/common/config/FieldConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL0ZpZWxkQ29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...va/org/apache/pinot/common/config/QueryConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1F1ZXJ5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/pinot/common/config/RoutingConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1JvdXRpbmdDb25maWcuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/pinot/common/http/MultiGetRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaHR0cC9NdWx0aUdldFJlcXVlc3QuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/pinot/common/lineage/SegmentGroup.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbGluZWFnZS9TZWdtZW50R3JvdXAuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| ... and [549 more](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=footer). Last update [d289db8...5cd5228](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5168: Refactor
DistinctQueriesTest to cover more scenarios
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull
request #5168: Refactor DistinctQueriesTest to cover more scenarios
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168#discussion_r401231445
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/reduce/DistinctDataTableReducer.java
##########
@@ -108,7 +108,7 @@ public void reduceAndSetResults(String tableName, DataSchema dataSchema,
if (mergedIntermediateResult == null) {
mergedIntermediateResult = intermediateResultToMerge;
} else {
- _aggregationFunction.merge(mergedIntermediateResult, intermediateResultToMerge);
+ mergedIntermediateResult = _aggregationFunction.merge(mergedIntermediateResult, intermediateResultToMerge);
Review comment:
Yes, and we should not make the assumption that the second result is merged into the first (for Double intermediateResult, that is not even possible)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #5168:
Refactor DistinctQueriesTest to cover more scenarios
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #5168: Refactor DistinctQueriesTest to cover more scenarios
URL: https://github.com/apache/incubator-pinot/pull/5168#issuecomment-601551841
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=h1) Report
> Merging [#5168](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1f1baf85fb318caa48b3b207e032946ddd5de11c&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `71.57%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5168 +/- ##
============================================
- Coverage 65.90% 65.87% -0.04%
Complexity 12 12
============================================
Files 1052 1055 +3
Lines 54170 54097 -73
Branches 8078 8047 -31
============================================
- Hits 35702 35636 -66
+ Misses 15819 15815 -4
+ Partials 2649 2646 -3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `76.66% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [.../BrokerResourceOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclJlc291cmNlT25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `55.81% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [.../pinot/broker/broker/helix/HelixBrokerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0hlbGl4QnJva2VyU3RhcnRlci5qYXZh) | `71.97% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...thandler/SingleConnectionBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvU2luZ2xlQ29ubmVjdGlvbkJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `92.68% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...rg/apache/pinot/broker/routing/RoutingManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9Sb3V0aW5nTWFuYWdlci5qYXZh) | `78.84% <ø> (-2.31%)` | `0.00 <0.00> (ø)` | |
| [...ting/instanceselector/InstanceSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3JGYWN0b3J5LmphdmE=) | `71.42% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...er/routing/segmentpruner/SegmentPrunerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1NlZ21lbnRQcnVuZXJGYWN0b3J5LmphdmE=) | `83.33% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...outing/segmentselector/SegmentSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50c2VsZWN0b3IvU2VnbWVudFNlbGVjdG9yRmFjdG9yeS5qYXZh) | `60.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...oker/routing/timeboundary/TimeBoundaryManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy90aW1lYm91bmRhcnkvVGltZUJvdW5kYXJ5TWFuYWdlci5qYXZh) | `87.50% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...mmon/assignment/InstanceAssignmentConfigUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZUFzc2lnbm1lbnRDb25maWdVdGlscy5qYXZh) | `67.50% <ø> (ø)` | `0.00 <0.00> (?)` | |
| ... and [170 more](https://codecov.io/gh/apache/incubator-pinot/pull/5168/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=footer). Last update [00fcb1d...65a7042](https://codecov.io/gh/apache/incubator-pinot/pull/5168?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org