You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/04/21 07:30:09 UTC

[GitHub] [druid] clintropolis opened a new pull request #9730: fill out missing test coverage for druid-datasketches postaggs

clintropolis opened a new pull request #9730:
URL: https://github.com/apache/druid/pull/9730


   Adds additional tests to bring all `PostAggregator` implementations in `druid-datasketches` extension to near 100% coverage (i got lazy and didn't add tests for some of the boring comparators). I replaced some `equals` and `hashCode` implementations which allowed subclasses of different types to test as equal since I don't think that is what we really want, with intellij generated versions.
   
   More PRs to follow to do the same for other `PostAggregator` implementations.


----------------------------------------------------------------
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



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


[GitHub] [druid] lgtm-com[bot] commented on issue #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on issue #9730:
URL: https://github.com/apache/druid/pull/9730#issuecomment-617050666


   This pull request **fixes 2 alerts** when merging fca98370c5fd9bcd884804207e26dea3d890f6f6 into b9ad250c009d7b689dcabf90c855e0d1955c5e3e - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-43b7355af57a45bc472e9bd916f3cf28492d3954)
   
   **fixed alerts:**
   
   * 2 for Useless null check


----------------------------------------------------------------
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



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


[GitHub] [druid] stale[bot] commented on pull request #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #9730:
URL: https://github.com/apache/druid/pull/9730#issuecomment-660442673


   This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.
   


----------------------------------------------------------------
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



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


[GitHub] [druid] lgtm-com[bot] commented on pull request #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #9730:
URL: https://github.com/apache/druid/pull/9730#issuecomment-666981901


   This pull request **fixes 2 alerts** when merging 7361ed660abdb32b5ec98712f50b8f6dbad91a83 into 646fa84d043a075e64efa0d65205a17b066204e9 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-7c96c82180ef5c3a5259a4a614b0c51ff75359d0)
   
   **fixed alerts:**
   
   * 2 for Useless null check


----------------------------------------------------------------
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



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


[GitHub] [druid] lgtm-com[bot] commented on pull request #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #9730:
URL: https://github.com/apache/druid/pull/9730#issuecomment-667068849


   This pull request **fixes 2 alerts** when merging d966c6e747f2f6072ec24b6592a67c78fe8db6d9 into 646fa84d043a075e64efa0d65205a17b066204e9 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-d7d4f9e514d7e699c5cac65e9d216024214cbdb6)
   
   **fixed alerts:**
   
   * 2 for Useless null check


----------------------------------------------------------------
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



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


[GitHub] [druid] lgtm-com[bot] commented on pull request #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #9730:
URL: https://github.com/apache/druid/pull/9730#issuecomment-630546740


   This pull request **fixes 2 alerts** when merging 5760a18628c973febafa71fee21ab99457f9cb1a into 82e5b0573efffd6f87ce4aead6a05b0c1d7381c7 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-72a8465b0435176e96aa6fc5f311b86a3ab99a57)
   
   **fixed alerts:**
   
   * 2 for Useless null check


----------------------------------------------------------------
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



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


[GitHub] [druid] clintropolis commented on a change in pull request #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9730:
URL: https://github.com/apache/druid/pull/9730#discussion_r463428144



##########
File path: extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java
##########
@@ -56,7 +56,7 @@ public SketchSetPostAggregator(
     Util.checkIfPowerOf2(this.maxSketchSize, "size");
 
     if (fields.size() <= 1) {
-      throw new IAE("Illegal number of fields[%s], must be > 1", fields.size());
+      throw new IAE("Illegal number of fields[%s], must be >= 1", fields.size());

Review comment:
       oops, fixed :+1:




----------------------------------------------------------------
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



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


[GitHub] [druid] lgtm-com[bot] commented on pull request #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #9730:
URL: https://github.com/apache/druid/pull/9730#issuecomment-666932614


   This pull request **fixes 2 alerts** when merging 280883da5815db7c9e6c4081a12c857bd7bda7d1 into 646fa84d043a075e64efa0d65205a17b066204e9 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-68098d0a0daa8da6cc6719ac1377ededb2fe27ad)
   
   **fixed alerts:**
   
   * 2 for Useless null check


----------------------------------------------------------------
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



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


[GitHub] [druid] gianm merged pull request #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
gianm merged pull request #9730:
URL: https://github.com/apache/druid/pull/9730


   


----------------------------------------------------------------
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



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


[GitHub] [druid] gianm commented on a change in pull request #9730: fill out missing test coverage for druid-datasketches postaggs

Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9730:
URL: https://github.com/apache/druid/pull/9730#discussion_r463423772



##########
File path: extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java
##########
@@ -56,7 +56,7 @@ public SketchSetPostAggregator(
     Util.checkIfPowerOf2(this.maxSketchSize, "size");
 
     if (fields.size() <= 1) {
-      throw new IAE("Illegal number of fields[%s], must be > 1", fields.size());
+      throw new IAE("Illegal number of fields[%s], must be >= 1", fields.size());

Review comment:
       The old message was right.




----------------------------------------------------------------
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



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