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 2021/05/15 18:32:39 UTC

[GitHub] [incubator-pinot] amrishlal opened a new pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

amrishlal opened a new pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927


   ## Description
   This PR does type casting of numerical types only for GREATER_THAN, GREATER_THAN_OR_EQUAL, LESS_THAN, and LESS_THAN_OR_EQUAL operators. A previous PR (see #6811) did numerical type casting for EQ and NOT_EQ operators. Future PRs will extend numerical type conversion to include other operators (IN, NOT_INT, and HAVING clause).
   
   Queries with predicates that compare a column expression of one numerical type (INT for example) with a literal of another numerical type (DOUBLE for example) will fail to evaluate on the server side:
   
   ```
   SELECT count(*) FROM testTable WHERE intColumn < 1.0
   SELECT count(*) FROM testTable WHERE intColumn <= 1.0
   SELECT count(*) FROM testTable WHERE intColumn > 15.1
   SELECT count(*) FROM testTable WHERE intColumn >= 15.1
   ```
   
   This happens because we currently don't support upcasting and downcasting of numerical predicates with different data types. To solve this problem, we rewrite the predicate on the Broker side to an equivalent predicate whose literal type is same as the column expression type before the query is send to the server for evaluation. Also, there are some cases where we can tell whether the filter will always evaluate to TRUE or FALSE. If FilterOptimizer determines that the filter will always evaluate to FALSE, the Broker will return an empty result back to the user without server-side query evaluation. Similarly, if FilterOptimizer determines that the filter will always evaluate to TRUE, the Broker will remove the WHERE clause before sending the query to the server.
   
   **Example 1: Simple conversion**
   The query `SELECT count(*) FROM testTable WHERE intColumn < 1.0` will be rewritten to `SELECT count(*) FROM testTable WHERE intColumn < 1` before server-side query evaluation.
   
   **Example 2: Predicate precomputed to false**
   The query `SELECT count(*) FROM testTable WHERE intColumn > 3000000000.5` will always evaluate to FALSE since no integer column can have decimal value greater than 3000000000.5. In this case the FilterOptimizer will rewrite the query to `SELECT count(*) FROM testTable WHERE FALSE` and Broker will send an empty response back to the user without server-side query evaluation.
   
   **Example 3: Predicate precomputed to true**
   The query `SELECT count(*) FROM testTable WHERE intColumn <= 3000000000.5` will always evaluate to TRUE since an integer column will always have values than 3000000000.5. In this case the FilterOptimizer will rewrite the query to `SELECT count(*) FROM testTable WHERE TRUE` and Broker will send an empty response back to the user without server-side query evaluation.
   
   **Example 4: Predicate operator modified**
   The query `SELECT count(*) FROM testTable WHERE intColumn < 300.5` will be rewritten to `SELECT count(*) FROM testTable WHERE intColumn <= 300`. Note that the interger counterpart (300) of a decimal value (300.5) is always less than the original value. Hence operator '<' is replaced with operator '<='.
   
   Similarly, the query `SELECT count(*) FROM testTable WHERE intColumn < -300.5` will be rewritten to `SELECT count(*) FROM testTable WHERE intColumn <= -300`
   
   The query `SELECT count(*) FROM testTable WHERE intColumn > -300.5` will be rewritten to `SELECT count(*) FROM testTable WHERE intColumn > -300`, and so on...
   
   ## Upgrade Notes
   Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
   * [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
   
   Does this PR fix a zero-downtime upgrade introduced earlier?
   * [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
   
   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
   <!-- If you have tagged this as either backward-incompat or release-notes,
   you MUST add text here that you would like to see appear in release notes of the
   next release. -->
   
   <!-- If you have a series of commits adding or enabling a feature, then
   add this section only in final commit that marks the feature completed.
   Refer to earlier release notes to see examples of text.
   -->
   ## Documentation
   <!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
   See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
   -->
   


-- 
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 #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6927](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c35fc76) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/59e342df9ba7895e1d404c7f681b7c8ab664d7de?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (59e342d) will **decrease** coverage by `30.40%`.
   > The diff coverage is `39.28%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6927/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #6927       +/-   ##
   =============================================
   - Coverage     73.49%   43.09%   -30.41%     
   + Complexity       12        7        -5     
   =============================================
     Files          1432     1432               
     Lines         70603    70668       +65     
     Branches      10206    10230       +24     
   =============================================
   - Hits          51892    30455    -21437     
   - Misses        15277    37584    +22307     
   + Partials       3434     2629      -805     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `43.09% <39.28%> (-0.15%)` | `7.00 <0.00> (ø)` | |
   | unittests | `?` | `?` | |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `43.39% <39.28%> (-44.91%)` | `0.00 <0.00> (ø)` | |
   | [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...a/org/apache/pinot/spi/data/DateTimeFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EYXRlVGltZUZpZWxkU3BlYy5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...a/org/apache/pinot/spi/env/PinotConfiguration.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZW52L1Bpbm90Q29uZmlndXJhdGlvbi5qYXZh) | `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/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#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/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...org/apache/pinot/spi/config/table/QuotaConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1F1b3RhQ29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...org/apache/pinot/spi/config/tenant/TenantRole.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RlbmFudC9UZW5hbnRSb2xlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [897 more](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [59e342d...c35fc76](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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 #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6927](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (813fb56) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/59e342df9ba7895e1d404c7f681b7c8ab664d7de?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (59e342d) will **decrease** coverage by `0.05%`.
   > The diff coverage is `83.33%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6927/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6927      +/-   ##
   ============================================
   - Coverage     73.49%   73.44%   -0.06%     
     Complexity       12       12              
   ============================================
     Files          1432     1431       -1     
     Lines         70603    70681      +78     
     Branches      10206    10244      +38     
   ============================================
   + Hits          51892    51910      +18     
   - Misses        15277    15314      +37     
   - Partials       3434     3457      +23     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `43.03% <39.28%> (-0.20%)` | `7.00 <0.00> (ø)` | |
   | unittests | `65.49% <83.33%> (+0.07%)` | `12.00 <0.00> (ø)` | |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `86.79% <83.33%> (-1.51%)` | `0.00 <0.00> (ø)` | |
   | [...data/manager/realtime/SegmentCommitterFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU2VnbWVudENvbW1pdHRlckZhY3RvcnkuamF2YQ==) | `64.70% <0.00%> (-35.30%)` | `0.00% <0.00%> (ø%)` | |
   | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `28.57% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
   | [...readers/constant/ConstantMVForwardIndexReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvY29uc3RhbnQvQ29uc3RhbnRNVkZvcndhcmRJbmRleFJlYWRlci5qYXZh) | `14.28% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
   | [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `63.85% <0.00%> (-12.05%)` | `0.00% <0.00%> (ø%)` | |
   | [...altime/ServerSegmentCompletionProtocolHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL3JlYWx0aW1lL1NlcnZlclNlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2xIYW5kbGVyLmphdmE=) | `48.54% <0.00%> (-8.74%)` | `0.00% <0.00%> (ø%)` | |
   | [...er/api/resources/LLCSegmentCompletionHandlers.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0xMQ1NlZ21lbnRDb21wbGV0aW9uSGFuZGxlcnMuamF2YQ==) | `51.16% <0.00%> (-8.38%)` | `0.00% <0.00%> (ø%)` | |
   | [...e/data/manager/realtime/SplitSegmentCommitter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU3BsaXRTZWdtZW50Q29tbWl0dGVyLmphdmE=) | `53.84% <0.00%> (-7.70%)` | `0.00% <0.00%> (ø%)` | |
   | [...nt/index/readers/ConstantValueBytesDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUJ5dGVzRGljdGlvbmFyeS5qYXZh) | `23.80% <0.00%> (-4.77%)` | `0.00% <0.00%> (ø%)` | |
   | [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9idWlsZGVyL0Jhc2VTaW5nbGVUcmVlQnVpbGRlci5qYXZh) | `88.39% <0.00%> (-4.47%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [43 more](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [59e342d...813fb56](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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] amrishlal commented on a change in pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640850582



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression lhs, Expression rhs, Sc
             break;
           }
         }
+      }
     }
     return equals;
   }
+
+  /**
+   * Rewrite expressions of form "column > literal", "column >= literal", "column < literal", and "column <= literal"
+   * to ensure that RHS literal is the same datatype as LHS column.
+   */
+  private static Expression rewriteRangeExpression(Expression range, FilterKind kind, Expression lhs, Expression rhs, Schema schema) {
+    // Get column data type.
+    FieldSpec.DataType dataType = schema.getFieldSpecFor(lhs.getIdentifier().getName()).getDataType();
+
+    switch (rhs.getLiteral().getSetField()) {
+      case SHORT_VALUE:
+      case INT_VALUE:
+        // No rewrites needed since SHORT and INT conversion to numeric column types (INT, LONG, FLOAT, and DOUBLE) is
+        // lossless and will be implicitly handled on the server side.
+        break;
+      case LONG_VALUE: {
+        long actual = rhs.getLiteral().getLongValue();
+        switch (dataType) {
+          case INT: {
+            int converted = (int) actual;
+            int comparison = Long.compare(actual, converted);
+            if (comparison > 0) {
+              // Literal value is greater than the bounds of INT. > and >= expressions will always be false because an
+              // INT column can never have a value greater than Integer.MAX_VALUE. < and <= expressions will always be
+              // true, because an INT column will always have values less than Integer.MIN_VALUE.
+              setExpressionToBoolean(range, kind == FilterKind.LESS_THAN || kind == FilterKind.LESS_THAN_OR_EQUAL);
+            } else if (comparison < 0) {
+              // Literal value is less than the bounds of INT. > and >= expressions will always be true because an
+              // INT column will always have a value greater than or equal to Integer.MIN_VALUE. < and <= expressions
+              // will always be false, because an INT column will never have values less than Integer.MIN_VALUE.
+              setExpressionToBoolean(range, kind == FilterKind.GREATER_THAN || kind == FilterKind.GREATER_THAN_OR_EQUAL);
+            } else {
+              // Long literal value falls within the bounds of INT column, server will successfully convert the literal
+              // value when needed.
+            }
+            break;
+          }
+          case FLOAT: {
+            // Since we are converting a long value to float, float value will never be out of bounds (i.e -Infinity
+            // or +Infinity).
+            float converted = (float) actual;
+            int comparison = BigDecimal.valueOf(actual).compareTo(BigDecimal.valueOf(converted));
+
+            // Rewrite range operator
+            rewriteRangeOperator(range, kind, comparison);
+
+            // Rewrite range literal
+            rhs.getLiteral().setDoubleValue(converted);
+            break;
+          }
+          case DOUBLE: {
+            // long to double conversion is always within bounds of double datatype, but the conversion can be lossy.
+            double converted = (double) actual;

Review comment:
       Added example in comment.




-- 
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] siddharthteotia commented on a change in pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640789829



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression lhs, Expression rhs, Sc
             break;
           }
         }
+      }
     }
     return equals;
   }
+
+  /**
+   * Rewrite expressions of form "column > literal", "column >= literal", "column < literal", and "column <= literal"
+   * to ensure that RHS literal is the same datatype as LHS column.
+   */
+  private static Expression rewriteRangeExpression(Expression range, FilterKind kind, Expression lhs, Expression rhs, Schema schema) {
+    // Get column data type.
+    FieldSpec.DataType dataType = schema.getFieldSpecFor(lhs.getIdentifier().getName()).getDataType();
+
+    switch (rhs.getLiteral().getSetField()) {
+      case SHORT_VALUE:
+      case INT_VALUE:
+        // No rewrites needed since SHORT and INT conversion to numeric column types (INT, LONG, FLOAT, and DOUBLE) is
+        // lossless and will be implicitly handled on the server side.
+        break;
+      case LONG_VALUE: {
+        long actual = rhs.getLiteral().getLongValue();
+        switch (dataType) {
+          case INT: {
+            int converted = (int) actual;
+            int comparison = Long.compare(actual, converted);
+            if (comparison > 0) {
+              // Literal value is greater than the bounds of INT. > and >= expressions will always be false because an
+              // INT column can never have a value greater than Integer.MAX_VALUE. < and <= expressions will always be
+              // true, because an INT column will always have values less than Integer.MIN_VALUE.
+              setExpressionToBoolean(range, kind == FilterKind.LESS_THAN || kind == FilterKind.LESS_THAN_OR_EQUAL);
+            } else if (comparison < 0) {
+              // Literal value is less than the bounds of INT. > and >= expressions will always be true because an
+              // INT column will always have a value greater than or equal to Integer.MIN_VALUE. < and <= expressions
+              // will always be false, because an INT column will never have values less than Integer.MIN_VALUE.
+              setExpressionToBoolean(range, kind == FilterKind.GREATER_THAN || kind == FilterKind.GREATER_THAN_OR_EQUAL);
+            } else {
+              // Long literal value falls within the bounds of INT column, server will successfully convert the literal
+              // value when needed.
+            }
+            break;
+          }
+          case FLOAT: {
+            // Since we are converting a long value to float, float value will never be out of bounds (i.e -Infinity
+            // or +Infinity).
+            float converted = (float) actual;

Review comment:
       How is this any different from the case for INT above where the LONG is being converted to INT




-- 
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] siddharthteotia commented on a change in pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640770793



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -80,22 +83,29 @@ public Expression optimize(Expression expression, @Nullable Schema schema) {
     List<Expression> operands = function.getOperands();
     String operator = function.getOperator();
     if (operator.equals(FilterKind.AND.name()) || operator.equals(FilterKind.OR.name())) {
-      // One of the operands may be an EQUALS or NOT_EQUALS function so recursively traverse the expression tree to see
-      // if we find an EQUALS or NOT_EQUALS function to rewrite.
+      // Recursively traverse the expression tree to find an operator node that can be rewritten.
       operands.forEach(operand -> optimize(operand, schema));
 
       // We have rewritten the child operands, so rewrite the parent if needed.
       return optimizeCurrent(expression);
-    } else if (operator.equals(FilterKind.EQUALS.name()) || operator.equals(FilterKind.NOT_EQUALS.name())) {
+    } else {
       // Verify that LHS is a numeric column and RHS is a numeric literal before rewriting.
       Expression lhs = operands.get(0);
       Expression rhs = operands.get(1);
       if (isNumericColumn(lhs, schema) && isNumericLiteral(rhs)) {
-        // Rewrite the expression.
-        return rewrite(expression, lhs, rhs, schema);
+        FilterKind kind = FilterKind.valueOf(operator);
+        switch (kind) {
+          case EQUALS:
+          case NOT_EQUALS:
+            return rewriteEqualsExpression(expression, kind, lhs, rhs, schema);
+          case GREATER_THAN:
+          case GREATER_THAN_OR_EQUAL:
+          case LESS_THAN:
+          case LESS_THAN_OR_EQUAL:
+            return rewriteRangeExpression(expression, kind, lhs, rhs, schema);

Review comment:
       Will we consider BETWEEN in the future ?

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression lhs, Expression rhs, Sc
             break;
           }
         }
+      }
     }
     return equals;
   }
+
+  /**
+   * Rewrite expressions of form "column > literal", "column >= literal", "column < literal", and "column <= literal"
+   * to ensure that RHS literal is the same datatype as LHS column.
+   */
+  private static Expression rewriteRangeExpression(Expression range, FilterKind kind, Expression lhs, Expression rhs, Schema schema) {
+    // Get column data type.
+    FieldSpec.DataType dataType = schema.getFieldSpecFor(lhs.getIdentifier().getName()).getDataType();
+
+    switch (rhs.getLiteral().getSetField()) {
+      case SHORT_VALUE:
+      case INT_VALUE:
+        // No rewrites needed since SHORT and INT conversion to numeric column types (INT, LONG, FLOAT, and DOUBLE) is
+        // lossless and will be implicitly handled on the server side.
+        break;
+      case LONG_VALUE: {
+        long actual = rhs.getLiteral().getLongValue();
+        switch (dataType) {
+          case INT: {
+            int converted = (int) actual;
+            int comparison = Long.compare(actual, converted);
+            if (comparison > 0) {
+              // Literal value is greater than the bounds of INT. > and >= expressions will always be false because an
+              // INT column can never have a value greater than Integer.MAX_VALUE. < and <= expressions will always be
+              // true, because an INT column will always have values less than Integer.MIN_VALUE.

Review comment:
       typo ?
   
   because an INT column will always have values **bigger** than Integer.MIN_VALUE and **less** than Integer.MAX_VALUE
   




-- 
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] amrishlal commented on a change in pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640852691



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression lhs, Expression rhs, Sc
             break;
           }
         }
+      }
     }
     return equals;
   }
+
+  /**
+   * Rewrite expressions of form "column > literal", "column >= literal", "column < literal", and "column <= literal"
+   * to ensure that RHS literal is the same datatype as LHS column.
+   */
+  private static Expression rewriteRangeExpression(Expression range, FilterKind kind, Expression lhs, Expression rhs, Schema schema) {
+    // Get column data type.
+    FieldSpec.DataType dataType = schema.getFieldSpecFor(lhs.getIdentifier().getName()).getDataType();
+
+    switch (rhs.getLiteral().getSetField()) {
+      case SHORT_VALUE:
+      case INT_VALUE:
+        // No rewrites needed since SHORT and INT conversion to numeric column types (INT, LONG, FLOAT, and DOUBLE) is
+        // lossless and will be implicitly handled on the server side.
+        break;
+      case LONG_VALUE: {
+        long actual = rhs.getLiteral().getLongValue();
+        switch (dataType) {
+          case INT: {
+            int converted = (int) actual;
+            int comparison = Long.compare(actual, converted);
+            if (comparison > 0) {
+              // Literal value is greater than the bounds of INT. > and >= expressions will always be false because an
+              // INT column can never have a value greater than Integer.MAX_VALUE. < and <= expressions will always be
+              // true, because an INT column will always have values less than Integer.MIN_VALUE.
+              setExpressionToBoolean(range, kind == FilterKind.LESS_THAN || kind == FilterKind.LESS_THAN_OR_EQUAL);
+            } else if (comparison < 0) {
+              // Literal value is less than the bounds of INT. > and >= expressions will always be true because an
+              // INT column will always have a value greater than or equal to Integer.MIN_VALUE. < and <= expressions
+              // will always be false, because an INT column will never have values less than Integer.MIN_VALUE.
+              setExpressionToBoolean(range, kind == FilterKind.GREATER_THAN || kind == FilterKind.GREATER_THAN_OR_EQUAL);
+            } else {
+              // Long literal value falls within the bounds of INT column, server will successfully convert the literal
+              // value when needed.
+            }
+            break;
+          }
+          case FLOAT: {
+            // Since we are converting a long value to float, float value will never be out of bounds (i.e -Infinity
+            // or +Infinity).
+            float converted = (float) actual;

Review comment:
       When an out of bounds value is converted to int, the int value becomes either Integer.MIN_VALUE or Integer.MAX_VALUE. However, when an out of bounds value is converted to float, the the float value becomes -Infinity or +Infinity. The code difference is mainly because of 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] amrishlal commented on a change in pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640840045



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -80,22 +83,29 @@ public Expression optimize(Expression expression, @Nullable Schema schema) {
     List<Expression> operands = function.getOperands();
     String operator = function.getOperator();
     if (operator.equals(FilterKind.AND.name()) || operator.equals(FilterKind.OR.name())) {
-      // One of the operands may be an EQUALS or NOT_EQUALS function so recursively traverse the expression tree to see
-      // if we find an EQUALS or NOT_EQUALS function to rewrite.
+      // Recursively traverse the expression tree to find an operator node that can be rewritten.
       operands.forEach(operand -> optimize(operand, schema));
 
       // We have rewritten the child operands, so rewrite the parent if needed.
       return optimizeCurrent(expression);
-    } else if (operator.equals(FilterKind.EQUALS.name()) || operator.equals(FilterKind.NOT_EQUALS.name())) {
+    } else {
       // Verify that LHS is a numeric column and RHS is a numeric literal before rewriting.
       Expression lhs = operands.get(0);
       Expression rhs = operands.get(1);
       if (isNumericColumn(lhs, schema) && isNumericLiteral(rhs)) {
-        // Rewrite the expression.
-        return rewrite(expression, lhs, rhs, schema);
+        FilterKind kind = FilterKind.valueOf(operator);
+        switch (kind) {
+          case EQUALS:
+          case NOT_EQUALS:
+            return rewriteEqualsExpression(expression, kind, lhs, rhs, schema);
+          case GREATER_THAN:
+          case GREATER_THAN_OR_EQUAL:
+          case LESS_THAN:
+          case LESS_THAN_OR_EQUAL:
+            return rewriteRangeExpression(expression, kind, lhs, rhs, schema);

Review comment:
       Added a TODO comment for doing numerical type conversion for BETWEEN, IN, and NOT IN operators.




-- 
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 #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6927](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c35fc76) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/59e342df9ba7895e1d404c7f681b7c8ab664d7de?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (59e342d) will **decrease** coverage by `30.40%`.
   > The diff coverage is `39.28%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6927/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #6927       +/-   ##
   =============================================
   - Coverage     73.49%   43.09%   -30.41%     
   + Complexity       12        7        -5     
   =============================================
     Files          1432     1432               
     Lines         70603    70668       +65     
     Branches      10206    10230       +24     
   =============================================
   - Hits          51892    30455    -21437     
   - Misses        15277    37584    +22307     
   + Partials       3434     2629      -805     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `43.09% <39.28%> (-0.15%)` | `7.00 <0.00> (ø)` | |
   | unittests | `?` | `?` | |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `43.39% <39.28%> (-44.91%)` | `0.00 <0.00> (ø)` | |
   | [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...a/org/apache/pinot/spi/data/DateTimeFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EYXRlVGltZUZpZWxkU3BlYy5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...a/org/apache/pinot/spi/env/PinotConfiguration.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZW52L1Bpbm90Q29uZmlndXJhdGlvbi5qYXZh) | `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/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#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/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...org/apache/pinot/spi/config/table/QuotaConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1F1b3RhQ29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...org/apache/pinot/spi/config/tenant/TenantRole.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RlbmFudC9UZW5hbnRSb2xlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [897 more](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [59e342d...c35fc76](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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 #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6927](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7d180e4) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/59e342df9ba7895e1d404c7f681b7c8ab664d7de?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (59e342d) will **decrease** coverage by `0.04%`.
   > The diff coverage is `83.33%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6927/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6927      +/-   ##
   ============================================
   - Coverage     73.49%   73.45%   -0.05%     
     Complexity       12       12              
   ============================================
     Files          1432     1432              
     Lines         70603    70668      +65     
     Branches      10206    10230      +24     
   ============================================
   + Hits          51892    51911      +19     
   - Misses        15277    15304      +27     
   - Partials       3434     3453      +19     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `43.02% <39.28%> (-0.21%)` | `7.00 <0.00> (ø)` | |
   | unittests | `65.44% <83.33%> (+0.02%)` | `12.00 <0.00> (ø)` | |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `86.79% <83.33%> (-1.51%)` | `0.00 <0.00> (ø)` | |
   | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `28.57% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
   | [...readers/constant/ConstantMVForwardIndexReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvY29uc3RhbnQvQ29uc3RhbnRNVkZvcndhcmRJbmRleFJlYWRlci5qYXZh) | `14.28% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
   | [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `67.46% <0.00%> (-8.44%)` | `0.00% <0.00%> (ø%)` | |
   | [...er/starter/helix/SegmentMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9TZWdtZW50TWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `69.33% <0.00%> (-6.67%)` | `0.00% <0.00%> (ø%)` | |
   | [...core/query/executor/ServerQueryExecutorV1Impl.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9leGVjdXRvci9TZXJ2ZXJRdWVyeUV4ZWN1dG9yVjFJbXBsLmphdmE=) | `79.45% <0.00%> (-4.87%)` | `0.00% <0.00%> (ø%)` | |
   | [...nt/index/readers/ConstantValueBytesDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUJ5dGVzRGljdGlvbmFyeS5qYXZh) | `23.80% <0.00%> (-4.77%)` | `0.00% <0.00%> (ø%)` | |
   | [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9idWlsZGVyL0Jhc2VTaW5nbGVUcmVlQnVpbGRlci5qYXZh) | `88.39% <0.00%> (-4.47%)` | `0.00% <0.00%> (ø%)` | |
   | [...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `55.31% <0.00%> (-4.26%)` | `0.00% <0.00%> (ø%)` | |
   | [...pinot/core/query/request/context/TimerContext.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9yZXF1ZXN0L2NvbnRleHQvVGltZXJDb250ZXh0LmphdmE=) | `91.66% <0.00%> (-4.17%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [18 more](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [59e342d...7d180e4](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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 #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6927](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (813fb56) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/59e342df9ba7895e1d404c7f681b7c8ab664d7de?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (59e342d) will **decrease** coverage by `8.00%`.
   > The diff coverage is `83.33%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6927/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6927      +/-   ##
   ============================================
   - Coverage     73.49%   65.49%   -8.01%     
     Complexity       12       12              
   ============================================
     Files          1432     1431       -1     
     Lines         70603    70681      +78     
     Branches      10206    10244      +38     
   ============================================
   - Hits          51892    46292    -5600     
   - Misses        15277    21072    +5795     
   + Partials       3434     3317     -117     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `?` | `?` | |
   | unittests | `65.49% <83.33%> (+0.07%)` | `12.00 <0.00> (ø)` | |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `86.79% <83.33%> (-1.51%)` | `0.00 <0.00> (ø)` | |
   | [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/minion/executor/MinionTaskZkMetadataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhlY3V0b3IvTWluaW9uVGFza1prTWV0YWRhdGFNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [354 more](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [59e342d...813fb56](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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] siddharthteotia commented on a change in pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640777126



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression lhs, Expression rhs, Sc
             break;
           }
         }
+      }
     }
     return equals;
   }
+
+  /**
+   * Rewrite expressions of form "column > literal", "column >= literal", "column < literal", and "column <= literal"
+   * to ensure that RHS literal is the same datatype as LHS column.
+   */
+  private static Expression rewriteRangeExpression(Expression range, FilterKind kind, Expression lhs, Expression rhs, Schema schema) {
+    // Get column data type.
+    FieldSpec.DataType dataType = schema.getFieldSpecFor(lhs.getIdentifier().getName()).getDataType();
+
+    switch (rhs.getLiteral().getSetField()) {
+      case SHORT_VALUE:
+      case INT_VALUE:
+        // No rewrites needed since SHORT and INT conversion to numeric column types (INT, LONG, FLOAT, and DOUBLE) is
+        // lossless and will be implicitly handled on the server side.
+        break;
+      case LONG_VALUE: {
+        long actual = rhs.getLiteral().getLongValue();
+        switch (dataType) {
+          case INT: {
+            int converted = (int) actual;
+            int comparison = Long.compare(actual, converted);
+            if (comparison > 0) {
+              // Literal value is greater than the bounds of INT. > and >= expressions will always be false because an
+              // INT column can never have a value greater than Integer.MAX_VALUE. < and <= expressions will always be
+              // true, because an INT column will always have values less than Integer.MIN_VALUE.
+              setExpressionToBoolean(range, kind == FilterKind.LESS_THAN || kind == FilterKind.LESS_THAN_OR_EQUAL);
+            } else if (comparison < 0) {
+              // Literal value is less than the bounds of INT. > and >= expressions will always be true because an
+              // INT column will always have a value greater than or equal to Integer.MIN_VALUE. < and <= expressions
+              // will always be false, because an INT column will never have values less than Integer.MIN_VALUE.
+              setExpressionToBoolean(range, kind == FilterKind.GREATER_THAN || kind == FilterKind.GREATER_THAN_OR_EQUAL);
+            } else {
+              // Long literal value falls within the bounds of INT column, server will successfully convert the literal
+              // value when needed.
+            }
+            break;
+          }
+          case FLOAT: {
+            // Since we are converting a long value to float, float value will never be out of bounds (i.e -Infinity
+            // or +Infinity).
+            float converted = (float) actual;
+            int comparison = BigDecimal.valueOf(actual).compareTo(BigDecimal.valueOf(converted));
+
+            // Rewrite range operator
+            rewriteRangeOperator(range, kind, comparison);
+
+            // Rewrite range literal
+            rhs.getLiteral().setDoubleValue(converted);
+            break;
+          }
+          case DOUBLE: {
+            // long to double conversion is always within bounds of double datatype, but the conversion can be lossy.
+            double converted = (double) actual;

Review comment:
       I don't think we were able to prove this earlier when we did a small testing exercise offline. Unless we can give an example of what is meant by lossy, I don't think we should add this comment




-- 
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 #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6927](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d95c953) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0b5dcb7aed1b85ab26345ab6c1076ccb88358f4a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0b5dcb7) will **decrease** coverage by `7.84%`.
   > The diff coverage is `83.33%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6927/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6927      +/-   ##
   ============================================
   - Coverage     73.39%   65.54%   -7.85%     
     Complexity       12       12              
   ============================================
     Files          1441     1441              
     Lines         71450    71515      +65     
     Branches      10354    10378      +24     
   ============================================
   - Hits          52438    46876    -5562     
   - Misses        15487    21240    +5753     
   + Partials       3525     3399     -126     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration | `?` | |
   | unittests | `65.54% <83.33%> (+0.05%)` | :arrow_up: |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `84.27% <83.33%> (-2.96%)` | :arrow_down: |
   | [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...t/minion/executor/MinionTaskZkMetadataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhlY3V0b3IvTWluaW9uVGFza1prTWV0YWRhdGFNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [336 more](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0b5dcb7...d95c953](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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] amrishlal commented on a change in pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640840142



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression lhs, Expression rhs, Sc
             break;
           }
         }
+      }
     }
     return equals;
   }
+
+  /**
+   * Rewrite expressions of form "column > literal", "column >= literal", "column < literal", and "column <= literal"
+   * to ensure that RHS literal is the same datatype as LHS column.
+   */
+  private static Expression rewriteRangeExpression(Expression range, FilterKind kind, Expression lhs, Expression rhs, Schema schema) {
+    // Get column data type.
+    FieldSpec.DataType dataType = schema.getFieldSpecFor(lhs.getIdentifier().getName()).getDataType();
+
+    switch (rhs.getLiteral().getSetField()) {
+      case SHORT_VALUE:
+      case INT_VALUE:
+        // No rewrites needed since SHORT and INT conversion to numeric column types (INT, LONG, FLOAT, and DOUBLE) is
+        // lossless and will be implicitly handled on the server side.
+        break;
+      case LONG_VALUE: {
+        long actual = rhs.getLiteral().getLongValue();
+        switch (dataType) {
+          case INT: {
+            int converted = (int) actual;
+            int comparison = Long.compare(actual, converted);
+            if (comparison > 0) {
+              // Literal value is greater than the bounds of INT. > and >= expressions will always be false because an
+              // INT column can never have a value greater than Integer.MAX_VALUE. < and <= expressions will always be
+              // true, because an INT column will always have values less than Integer.MIN_VALUE.

Review comment:
       Fixed.

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression lhs, Expression rhs, Sc
             break;
           }
         }
+      }
     }
     return equals;
   }
+
+  /**
+   * Rewrite expressions of form "column > literal", "column >= literal", "column < literal", and "column <= literal"
+   * to ensure that RHS literal is the same datatype as LHS column.
+   */

Review comment:
       Added a TODO comment for doing numerical type conversion for BETWEEN, IN, and NOT IN operators.




-- 
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] siddharthteotia commented on a change in pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6927:
URL: https://github.com/apache/incubator-pinot/pull/6927#discussion_r640771794



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java
##########
@@ -271,7 +242,211 @@ private Expression rewrite(Expression equals, Expression lhs, Expression rhs, Sc
             break;
           }
         }
+      }
     }
     return equals;
   }
+
+  /**
+   * Rewrite expressions of form "column > literal", "column >= literal", "column < literal", and "column <= literal"
+   * to ensure that RHS literal is the same datatype as LHS column.
+   */

Review comment:
       Let's add a TODO for BETWEEN ?




-- 
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 #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6927](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7d180e4) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/59e342df9ba7895e1d404c7f681b7c8ab664d7de?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (59e342d) will **decrease** coverage by `0.04%`.
   > The diff coverage is `83.33%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6927/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6927      +/-   ##
   ============================================
   - Coverage     73.49%   73.45%   -0.05%     
     Complexity       12       12              
   ============================================
     Files          1432     1432              
     Lines         70603    70668      +65     
     Branches      10206    10230      +24     
   ============================================
   + Hits          51892    51911      +19     
   - Misses        15277    15304      +27     
   - Partials       3434     3453      +19     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `43.02% <39.28%> (-0.21%)` | `7.00 <0.00> (ø)` | |
   | unittests | `65.44% <83.33%> (+0.02%)` | `12.00 <0.00> (ø)` | |
   
   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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `86.79% <83.33%> (-1.51%)` | `0.00 <0.00> (ø)` | |
   | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `28.57% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
   | [...readers/constant/ConstantMVForwardIndexReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvY29uc3RhbnQvQ29uc3RhbnRNVkZvcndhcmRJbmRleFJlYWRlci5qYXZh) | `14.28% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
   | [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `67.46% <0.00%> (-8.44%)` | `0.00% <0.00%> (ø%)` | |
   | [...er/starter/helix/SegmentMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9TZWdtZW50TWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `69.33% <0.00%> (-6.67%)` | `0.00% <0.00%> (ø%)` | |
   | [...core/query/executor/ServerQueryExecutorV1Impl.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9leGVjdXRvci9TZXJ2ZXJRdWVyeUV4ZWN1dG9yVjFJbXBsLmphdmE=) | `79.45% <0.00%> (-4.87%)` | `0.00% <0.00%> (ø%)` | |
   | [...nt/index/readers/ConstantValueBytesDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUJ5dGVzRGljdGlvbmFyeS5qYXZh) | `23.80% <0.00%> (-4.77%)` | `0.00% <0.00%> (ø%)` | |
   | [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9idWlsZGVyL0Jhc2VTaW5nbGVUcmVlQnVpbGRlci5qYXZh) | `88.39% <0.00%> (-4.47%)` | `0.00% <0.00%> (ø%)` | |
   | [...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `55.31% <0.00%> (-4.26%)` | `0.00% <0.00%> (ø%)` | |
   | [...pinot/core/query/request/context/TimerContext.java](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9yZXF1ZXN0L2NvbnRleHQvVGltZXJDb250ZXh0LmphdmE=) | `91.66% <0.00%> (-4.17%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [18 more](https://codecov.io/gh/apache/incubator-pinot/pull/6927/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [59e342d...7d180e4](https://codecov.io/gh/apache/incubator-pinot/pull/6927?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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] siddharthteotia merged pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   


-- 
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] amrishlal commented on pull request #6927: Normalize LHS and RHS numerical types for >, >=, <, and <= operators.

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


   @siddharthteotia @Jackie-Jiang Please 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.

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