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/06/16 02:04:58 UTC

[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #5570: Clean up the code for HAVING clause handling

Jackie-Jiang opened a new pull request #5570:
URL: https://github.com/apache/incubator-pinot/pull/5570


   ## Description
   - HAVING clause does not apply to PQL GROUP-BY semantic where each aggregation column is ordered individually
   - Clean up the code for HAVING clause handling which can generate wrong results
   - Stop accepting PQL queries with HAVING clause
   
   Will add back HAVING clause for SQL semantic after migrating from PQL to SQL.
   
   Does this PR otherwise need attention when creating release notes? Things to consider:
   - New configuration options
   - Deprecation of configurations
   - Signature changes to public methods/interfaces
   - New plugins added or old plugins removed
   * [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
   ## Release Notes
   PQL queries with HAVING clause will no longer be accepted.


----------------------------------------------------------------
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@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #5570: Clean up the code for HAVING clause handling

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #5570:
URL: https://github.com/apache/incubator-pinot/pull/5570#issuecomment-644955751


   @mayankshriv Added more release notes


----------------------------------------------------------------
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@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5570: Clean up the code for HAVING clause handling

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #5570:
URL: https://github.com/apache/incubator-pinot/pull/5570


   


----------------------------------------------------------------
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@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #5570: Clean up the code for HAVING clause handling

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #5570:
URL: https://github.com/apache/incubator-pinot/pull/5570#issuecomment-644553089


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5570?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d8764cc`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `46.29%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/5570/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/5570?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #5570   +/-   ##
   =========================================
     Coverage          ?   66.28%           
   =========================================
     Files             ?     1110           
     Lines             ?    57013           
     Branches          ?     8514           
   =========================================
     Hits              ?    37789           
     Misses            ?    16446           
     Partials          ?     2778           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #integrationtests | `45.05% <37.03%> (?)` | |
   | #unittests | `57.00% <44.44%> (?)` | |
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5570?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...pache/pinot/common/utils/request/RequestUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvcmVxdWVzdC9SZXF1ZXN0VXRpbHMuamF2YQ==) | `62.42% <ø> (ø)` | |
   | [.../org/apache/pinot/pql/parsers/Pql2AstListener.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QcWwyQXN0TGlzdGVuZXIuamF2YQ==) | `91.97% <0.00%> (ø)` | |
   | [...ava/org/apache/pinot/pql/parsers/Pql2Compiler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QcWwyQ29tcGlsZXIuamF2YQ==) | `88.15% <ø> (ø)` | |
   | [...e/pinot/pql/parsers/pql2/ast/PredicateAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9QcmVkaWNhdGVBc3ROb2RlLmphdmE=) | `100.00% <ø> (ø)` | |
   | [...ers/pql2/ast/PredicateParenthesisGroupAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9QcmVkaWNhdGVQYXJlbnRoZXNpc0dyb3VwQXN0Tm9kZS5qYXZh) | `100.00% <ø> (ø)` | |
   | [...ache/pinot/pql/parsers/pql2/ast/SelectAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9TZWxlY3RBc3ROb2RlLmphdmE=) | `98.87% <ø> (ø)` | |
   | [...gregation/function/AggregationFunctionFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | `74.66% <ø> (ø)` | |
   | [...l/parsers/pql2/ast/RegexpLikePredicateAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9SZWdleHBMaWtlUHJlZGljYXRlQXN0Tm9kZS5qYXZh) | `50.00% <16.66%> (ø)` | |
   | [.../pql/parsers/pql2/ast/BetweenPredicateAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9CZXR3ZWVuUHJlZGljYXRlQXN0Tm9kZS5qYXZh) | `51.35% <20.00%> (ø)` | |
   | [...l/parsers/pql2/ast/ComparisonPredicateAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9Db21wYXJpc29uUHJlZGljYXRlQXN0Tm9kZS5qYXZh) | `50.49% <33.33%> (ø)` | |
   | ... and [1115 more](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5570?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/5570?src=pr&el=footer). Last update [d8764cc...0ac4b1b](https://codecov.io/gh/apache/incubator-pinot/pull/5570?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



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

[GitHub] [incubator-pinot] mayankshriv commented on pull request #5570: Clean up the code for HAVING clause handling

Posted by GitBox <gi...@apache.org>.
mayankshriv commented on pull request #5570:
URL: https://github.com/apache/incubator-pinot/pull/5570#issuecomment-644929997


   Add a bit more info in the release notes on why we are removing this.


----------------------------------------------------------------
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@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter commented on pull request #5570: Clean up the code for HAVING clause handling

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #5570:
URL: https://github.com/apache/incubator-pinot/pull/5570#issuecomment-644553089


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5570?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@d8764cc`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `46.29%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/5570/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/5570?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #5570   +/-   ##
   =========================================
     Coverage          ?   66.28%           
   =========================================
     Files             ?     1110           
     Lines             ?    57013           
     Branches          ?     8514           
   =========================================
     Hits              ?    37789           
     Misses            ?    16446           
     Partials          ?     2778           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #integrationtests | `45.05% <37.03%> (?)` | |
   | #unittests | `57.00% <44.44%> (?)` | |
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5570?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...pache/pinot/common/utils/request/RequestUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvcmVxdWVzdC9SZXF1ZXN0VXRpbHMuamF2YQ==) | `62.42% <ø> (ø)` | |
   | [.../org/apache/pinot/pql/parsers/Pql2AstListener.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QcWwyQXN0TGlzdGVuZXIuamF2YQ==) | `91.97% <0.00%> (ø)` | |
   | [...ava/org/apache/pinot/pql/parsers/Pql2Compiler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QcWwyQ29tcGlsZXIuamF2YQ==) | `88.15% <ø> (ø)` | |
   | [...e/pinot/pql/parsers/pql2/ast/PredicateAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9QcmVkaWNhdGVBc3ROb2RlLmphdmE=) | `100.00% <ø> (ø)` | |
   | [...ers/pql2/ast/PredicateParenthesisGroupAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9QcmVkaWNhdGVQYXJlbnRoZXNpc0dyb3VwQXN0Tm9kZS5qYXZh) | `100.00% <ø> (ø)` | |
   | [...ache/pinot/pql/parsers/pql2/ast/SelectAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9TZWxlY3RBc3ROb2RlLmphdmE=) | `98.87% <ø> (ø)` | |
   | [...gregation/function/AggregationFunctionFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | `74.66% <ø> (ø)` | |
   | [...l/parsers/pql2/ast/RegexpLikePredicateAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9SZWdleHBMaWtlUHJlZGljYXRlQXN0Tm9kZS5qYXZh) | `50.00% <16.66%> (ø)` | |
   | [.../pql/parsers/pql2/ast/BetweenPredicateAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9CZXR3ZWVuUHJlZGljYXRlQXN0Tm9kZS5qYXZh) | `51.35% <20.00%> (ø)` | |
   | [...l/parsers/pql2/ast/ComparisonPredicateAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9Db21wYXJpc29uUHJlZGljYXRlQXN0Tm9kZS5qYXZh) | `50.49% <33.33%> (ø)` | |
   | ... and [1115 more](https://codecov.io/gh/apache/incubator-pinot/pull/5570/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5570?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/5570?src=pr&el=footer). Last update [d8764cc...0ac4b1b](https://codecov.io/gh/apache/incubator-pinot/pull/5570?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



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