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 2022/02/27 19:01:48 UTC

[GitHub] [pinot] amrishlal opened a new pull request #8253: Fix string comparisons.

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


   ## Description
   A query such as `SELECT count(*) FROM mytable WHERE trim(OriginState) = DestState` currently throws exception, because internally the predicate gets rewritten to `MINUS(trim(OriginState), DestState) = 0` and MINUS function fails to evaluate STRING arguments. This PR modifies `StringPredicateFilterOptizer.java` to properly rewrite predicate to use STRCMP instead of MINUS (`STRCMP(trim(OriginState), DestState) = 0`).
   
   Note that the same issue was fixed for string columns in #7394. This PR is extending the original fix to include string functions as well.
   ## 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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] siddharthteotia merged pull request #8253: Fix string comparisons.

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


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] richardstartin commented on a change in pull request #8253: Fix string comparisons.

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8253:
URL: https://github.com/apache/pinot/pull/8253#discussion_r816041704



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -99,21 +104,37 @@ private static void replaceMinusWithCompareForStrings(Expression expression, Sch
     Function function = expression.getFunctionCall();
     String operator = function.getOperator();
     List<Expression> operands = function.getOperands();
-    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && isStringColumn(operands.get(0), schema)
-        && isStringColumn(operands.get(1), schema)) {
+    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && isString(operands.get(0), schema)
+        && isString(operands.get(1), schema)) {
       function.setOperator(STRCMP_OPERATOR_NAME);
     }
   }
 
-  /** @return true if expression is a column of string type. */
-  private static boolean isStringColumn(Expression expression, Schema schema) {
-    if (expression.getType() != ExpressionType.IDENTIFIER) {
-      // Expression is not a column.
-      return false;
+  /** @return true if expression is STRING column or a function that outputs STRING. */
+  private static boolean isString(Expression expression, Schema schema) {
+    ExpressionType expressionType = expression.getType();
+
+    if (expressionType == ExpressionType.IDENTIFIER) {
+      // Check if this is a STRING column.
+      String column = expression.getIdentifier().getName();
+      FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+      return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
     }
 
-    String column = expression.getIdentifier().getName();
-    FieldSpec fieldSpec = schema.getFieldSpecFor(column);
-    return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
+    // Check if the function returns STRING as output.
+    return (expressionType == ExpressionType.FUNCTION && STRING_OUTPUT_FUNCTIONS
+        .contains(expression.getFunctionCall().getOperator().toUpperCase()));
+  }
+
+  /** List of string functions that return STRING output. */
+  private static Set<String> getStringOutputFunctionList() {
+    Set<String> set = new HashSet<>();
+    Method[] methods = StringFunctions.class.getDeclaredMethods();
+    for (Method method : methods) {
+      if (method.getReturnType() == String.class) {
+        set.add(method.getName().toUpperCase());
+      }
+    }
+    return set;

Review comment:
       `@ScalarFunction` now has a `names` array parameter - so you need to read the names from the annotation to do this correctly.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] richardstartin commented on a change in pull request #8253: Fix string comparisons.

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8253:
URL: https://github.com/apache/pinot/pull/8253#discussion_r816133799



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -99,21 +104,37 @@ private static void replaceMinusWithCompareForStrings(Expression expression, Sch
     Function function = expression.getFunctionCall();
     String operator = function.getOperator();
     List<Expression> operands = function.getOperands();
-    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && isStringColumn(operands.get(0), schema)
-        && isStringColumn(operands.get(1), schema)) {
+    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && isString(operands.get(0), schema)
+        && isString(operands.get(1), schema)) {
       function.setOperator(STRCMP_OPERATOR_NAME);
     }
   }
 
-  /** @return true if expression is a column of string type. */
-  private static boolean isStringColumn(Expression expression, Schema schema) {
-    if (expression.getType() != ExpressionType.IDENTIFIER) {
-      // Expression is not a column.
-      return false;
+  /** @return true if expression is STRING column or a function that outputs STRING. */
+  private static boolean isString(Expression expression, Schema schema) {
+    ExpressionType expressionType = expression.getType();
+
+    if (expressionType == ExpressionType.IDENTIFIER) {
+      // Check if this is a STRING column.
+      String column = expression.getIdentifier().getName();
+      FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+      return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
     }
 
-    String column = expression.getIdentifier().getName();
-    FieldSpec fieldSpec = schema.getFieldSpecFor(column);
-    return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
+    // Check if the function returns STRING as output.
+    return (expressionType == ExpressionType.FUNCTION && STRING_OUTPUT_FUNCTIONS
+        .contains(expression.getFunctionCall().getOperator().toUpperCase()));
+  }
+
+  /** List of string functions that return STRING output. */
+  private static Set<String> getStringOutputFunctionList() {
+    Set<String> set = new HashSet<>();
+    Method[] methods = StringFunctions.class.getDeclaredMethods();
+    for (Method method : methods) {
+      if (method.getReturnType() == String.class) {
+        set.add(method.getName().toUpperCase());
+      }
+    }
+    return set;

Review comment:
       Sorry, allow me to clarify:
   
   ```java
       Set<String> set = new HashSet<>();
       Method[] methods = StringFunctions.class.getDeclaredMethods();
       for (Method method : methods) {
         if (method.getReturnType() == String.class) {
           if (method.isAnnotationPresent(ScalarFunction.class)) {
              ScalarFunction annotation = method.getAnnotation(ScalarFunction.class);
              for (String name : annotation.names()) {
                set.add(name.toUpperCase());
              }   
           }
           set.add(method.getName().toUpperCase());
         }
       }
       return set;
   ```
   
   This way your logic works even if there is an alias in use. Does that make sense?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] amrishlal commented on a change in pull request #8253: Fix string comparisons.

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



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -99,21 +104,37 @@ private static void replaceMinusWithCompareForStrings(Expression expression, Sch
     Function function = expression.getFunctionCall();
     String operator = function.getOperator();
     List<Expression> operands = function.getOperands();
-    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && isStringColumn(operands.get(0), schema)
-        && isStringColumn(operands.get(1), schema)) {
+    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && isString(operands.get(0), schema)
+        && isString(operands.get(1), schema)) {
       function.setOperator(STRCMP_OPERATOR_NAME);
     }
   }
 
-  /** @return true if expression is a column of string type. */
-  private static boolean isStringColumn(Expression expression, Schema schema) {
-    if (expression.getType() != ExpressionType.IDENTIFIER) {
-      // Expression is not a column.
-      return false;
+  /** @return true if expression is STRING column or a function that outputs STRING. */
+  private static boolean isString(Expression expression, Schema schema) {
+    ExpressionType expressionType = expression.getType();
+
+    if (expressionType == ExpressionType.IDENTIFIER) {
+      // Check if this is a STRING column.
+      String column = expression.getIdentifier().getName();
+      FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+      return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
     }
 
-    String column = expression.getIdentifier().getName();
-    FieldSpec fieldSpec = schema.getFieldSpecFor(column);
-    return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
+    // Check if the function returns STRING as output.
+    return (expressionType == ExpressionType.FUNCTION && STRING_OUTPUT_FUNCTIONS
+        .contains(expression.getFunctionCall().getOperator().toUpperCase()));
+  }
+
+  /** List of string functions that return STRING output. */
+  private static Set<String> getStringOutputFunctionList() {
+    Set<String> set = new HashSet<>();
+    Method[] methods = StringFunctions.class.getDeclaredMethods();
+    for (Method method : methods) {
+      if (method.getReturnType() == String.class) {
+        set.add(method.getName().toUpperCase());
+      }
+    }
+    return set;

Review comment:
       Wouldn't using @ScalarFunction return names of all scalar functions and how would the output type of the function be resolved? We just want String scalar functions and that too only the once that return a string as output. All the string scalar functions are declared in StringFunctions and hence this code.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] amrishlal commented on pull request #8253: Fix string comparisons.

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


   > Please remove the binary file added by accident.
   
   Done


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #8253: Fix string comparisons.

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


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8253?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 [#8253](https://codecov.io/gh/apache/pinot/pull/8253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16f8871) into [master](https://codecov.io/gh/apache/pinot/commit/0ac5f1fb8897c05ddb9bd059f849d2dc540f44d1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0ac5f1f) will **decrease** coverage by `6.65%`.
   > The diff coverage is `68.75%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/8253/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/pinot/pull/8253?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    #8253      +/-   ##
   ============================================
   - Coverage     70.69%   64.04%   -6.66%     
   - Complexity     4238     4242       +4     
   ============================================
     Files          1629     1586      -43     
     Lines         85215    83408    -1807     
     Branches      12830    12646     -184     
   ============================================
   - Hits          60243    53415    -6828     
   - Misses        20814    26151    +5337     
   + Partials       4158     3842     -316     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `66.99% <68.75%> (+0.04%)` | :arrow_up: |
   | unittests2 | `14.09% <0.00%> (-0.06%)` | :arrow_down: |
   
   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/pinot/pull/8253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvc3RhdGVtZW50L1N0cmluZ1ByZWRpY2F0ZUZpbHRlck9wdGltaXplci5qYXZh) | `89.36% <68.75%> (-2.75%)` | :arrow_down: |
   | [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8253/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/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ache/pinot/server/access/AccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL2FjY2Vzcy9BY2Nlc3NDb250cm9sRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...che/pinot/controller/util/TableMetadataReader.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1RhYmxlTWV0YWRhdGFSZWFkZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/8253/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: |
   | ... and [389 more](https://codecov.io/gh/apache/pinot/pull/8253/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/pinot/pull/8253?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/pinot/pull/8253?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 [0ac5f1f...16f8871](https://codecov.io/gh/apache/pinot/pull/8253?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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #8253: Fix string comparisons.

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


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8253?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 [#8253](https://codecov.io/gh/apache/pinot/pull/8253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16f8871) into [master](https://codecov.io/gh/apache/pinot/commit/0ac5f1fb8897c05ddb9bd059f849d2dc540f44d1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0ac5f1f) will **increase** coverage by `0.00%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/8253/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/pinot/pull/8253?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    #8253   +/-   ##
   =========================================
     Coverage     70.69%   70.69%           
   - Complexity     4238     4242    +4     
   =========================================
     Files          1629     1631    +2     
     Lines         85215    85290   +75     
     Branches      12830    12850   +20     
   =========================================
   + Hits          60243    60298   +55     
   - Misses        20814    20832   +18     
   - Partials       4158     4160    +2     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `28.81% <75.00%> (+0.06%)` | :arrow_up: |
   | integration2 | `27.56% <75.00%> (+0.05%)` | :arrow_up: |
   | unittests1 | `66.99% <68.75%> (+0.04%)` | :arrow_up: |
   | unittests2 | `14.09% <0.00%> (-0.06%)` | :arrow_down: |
   
   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/pinot/pull/8253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvc3RhdGVtZW50L1N0cmluZ1ByZWRpY2F0ZUZpbHRlck9wdGltaXplci5qYXZh) | `91.48% <75.00%> (-0.62%)` | :arrow_down: |
   | [...pinot/core/query/request/context/TimerContext.java](https://codecov.io/gh/apache/pinot/pull/8253/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%)` | :arrow_down: |
   | [...core/query/executor/ServerQueryExecutorV1Impl.java](https://codecov.io/gh/apache/pinot/pull/8253/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=) | `83.02% <0.00%> (-4.13%)` | :arrow_down: |
   | [...verter/stats/MutableNoDictionaryColStatistics.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9jb252ZXJ0ZXIvc3RhdHMvTXV0YWJsZU5vRGljdGlvbmFyeUNvbFN0YXRpc3RpY3MuamF2YQ==) | `40.90% <0.00%> (-4.10%)` | :arrow_down: |
   | [...gregation/function/StUnionAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9TdFVuaW9uQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `73.52% <0.00%> (-2.95%)` | :arrow_down: |
   | [.../loader/defaultcolumn/DefaultColumnStatistics.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9kZWZhdWx0Y29sdW1uL0RlZmF1bHRDb2x1bW5TdGF0aXN0aWNzLmphdmE=) | `73.07% <0.00%> (-2.93%)` | :arrow_down: |
   | [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `87.85% <0.00%> (-1.87%)` | :arrow_down: |
   | [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `67.51% <0.00%> (-1.83%)` | :arrow_down: |
   | [...ltime/converter/stats/MutableColumnStatistics.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9jb252ZXJ0ZXIvc3RhdHMvTXV0YWJsZUNvbHVtblN0YXRpc3RpY3MuamF2YQ==) | `53.22% <0.00%> (-1.78%)` | :arrow_down: |
   | [...apache/pinot/controller/api/upload/ZKOperator.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvdXBsb2FkL1pLT3BlcmF0b3IuamF2YQ==) | `74.80% <0.00%> (-1.58%)` | :arrow_down: |
   | ... and [33 more](https://codecov.io/gh/apache/pinot/pull/8253/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/pinot/pull/8253?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/pinot/pull/8253?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 [0ac5f1f...16f8871](https://codecov.io/gh/apache/pinot/pull/8253?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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] codecov-commenter edited a comment on pull request #8253: Fix string comparisons.

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


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8253?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 [#8253](https://codecov.io/gh/apache/pinot/pull/8253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16f8871) into [master](https://codecov.io/gh/apache/pinot/commit/0ac5f1fb8897c05ddb9bd059f849d2dc540f44d1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0ac5f1f) will **decrease** coverage by `1.02%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/8253/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/pinot/pull/8253?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    #8253      +/-   ##
   ============================================
   - Coverage     70.69%   69.66%   -1.03%     
   - Complexity     4238     4242       +4     
   ============================================
     Files          1629     1631       +2     
     Lines         85215    85290      +75     
     Branches      12830    12850      +20     
   ============================================
   - Hits          60243    59418     -825     
   - Misses        20814    21741     +927     
   + Partials       4158     4131      -27     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `28.81% <75.00%> (+0.06%)` | :arrow_up: |
   | integration2 | `?` | |
   | unittests1 | `66.99% <68.75%> (+0.04%)` | :arrow_up: |
   | unittests2 | `14.09% <0.00%> (-0.06%)` | :arrow_down: |
   
   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/pinot/pull/8253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvc3RhdGVtZW50L1N0cmluZ1ByZWRpY2F0ZUZpbHRlck9wdGltaXplci5qYXZh) | `91.48% <75.00%> (-0.62%)` | :arrow_down: |
   | [...t/core/plan/StreamingInstanceResponsePlanNode.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL1N0cmVhbWluZ0luc3RhbmNlUmVzcG9uc2VQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ore/operator/streaming/StreamingResponseUtils.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nUmVzcG9uc2VVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ager/realtime/PeerSchemeSplitSegmentCommitter.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUGVlclNjaGVtZVNwbGl0U2VnbWVudENvbW1pdHRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...he/pinot/core/plan/StreamingSelectionPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL1N0cmVhbWluZ1NlbGVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-88.89%)` | :arrow_down: |
   | [...ator/streaming/StreamingSelectionOnlyOperator.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nU2VsZWN0aW9uT25seU9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-87.81%)` | :arrow_down: |
   | [...re/query/reduce/SelectionOnlyStreamingReducer.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9yZWR1Y2UvU2VsZWN0aW9uT25seVN0cmVhbWluZ1JlZHVjZXIuamF2YQ==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/pinot/pull/8253/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%> (-80.86%)` | :arrow_down: |
   | [...ller/api/access/BasicAuthAccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvYWNjZXNzL0Jhc2ljQXV0aEFjY2Vzc0NvbnRyb2xGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
   | [...roker/requesthandler/GrpcBrokerRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvR3JwY0Jyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (-78.58%)` | :arrow_down: |
   | ... and [106 more](https://codecov.io/gh/apache/pinot/pull/8253/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/pinot/pull/8253?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/pinot/pull/8253?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 [0ac5f1f...16f8871](https://codecov.io/gh/apache/pinot/pull/8253?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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] codecov-commenter commented on pull request #8253: Fix string comparisons.

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


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8253?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 [#8253](https://codecov.io/gh/apache/pinot/pull/8253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16f8871) into [master](https://codecov.io/gh/apache/pinot/commit/0ac5f1fb8897c05ddb9bd059f849d2dc540f44d1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0ac5f1f) will **decrease** coverage by `56.60%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/8253/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/pinot/pull/8253?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    #8253       +/-   ##
   =============================================
   - Coverage     70.69%   14.09%   -56.61%     
   + Complexity     4238       81     -4157     
   =============================================
     Files          1629     1586       -43     
     Lines         85215    83408     -1807     
     Branches      12830    12646      -184     
   =============================================
   - Hits          60243    11754    -48489     
   - Misses        20814    70776    +49962     
   + Partials       4158      878     -3280     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `?` | |
   | unittests2 | `14.09% <0.00%> (-0.06%)` | :arrow_down: |
   
   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/pinot/pull/8253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvc3RhdGVtZW50L1N0cmluZ1ByZWRpY2F0ZUZpbHRlck9wdGltaXplci5qYXZh) | `0.00% <0.00%> (-92.11%)` | :arrow_down: |
   | [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/8253/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%)` | :arrow_down: |
   | [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/pinot/common/utils/StringUtil.java](https://codecov.io/gh/apache/pinot/pull/8253/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU3RyaW5nVXRpbC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [1299 more](https://codecov.io/gh/apache/pinot/pull/8253/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/pinot/pull/8253?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/pinot/pull/8253?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 [0ac5f1f...16f8871](https://codecov.io/gh/apache/pinot/pull/8253?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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] amrishlal commented on a change in pull request #8253: Fix string comparisons.

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



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -99,21 +104,37 @@ private static void replaceMinusWithCompareForStrings(Expression expression, Sch
     Function function = expression.getFunctionCall();
     String operator = function.getOperator();
     List<Expression> operands = function.getOperands();
-    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && isStringColumn(operands.get(0), schema)
-        && isStringColumn(operands.get(1), schema)) {
+    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && isString(operands.get(0), schema)
+        && isString(operands.get(1), schema)) {
       function.setOperator(STRCMP_OPERATOR_NAME);
     }
   }
 
-  /** @return true if expression is a column of string type. */
-  private static boolean isStringColumn(Expression expression, Schema schema) {
-    if (expression.getType() != ExpressionType.IDENTIFIER) {
-      // Expression is not a column.
-      return false;
+  /** @return true if expression is STRING column or a function that outputs STRING. */
+  private static boolean isString(Expression expression, Schema schema) {
+    ExpressionType expressionType = expression.getType();
+
+    if (expressionType == ExpressionType.IDENTIFIER) {
+      // Check if this is a STRING column.
+      String column = expression.getIdentifier().getName();
+      FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+      return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
     }
 
-    String column = expression.getIdentifier().getName();
-    FieldSpec fieldSpec = schema.getFieldSpecFor(column);
-    return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
+    // Check if the function returns STRING as output.
+    return (expressionType == ExpressionType.FUNCTION && STRING_OUTPUT_FUNCTIONS
+        .contains(expression.getFunctionCall().getOperator().toUpperCase()));
+  }
+
+  /** List of string functions that return STRING output. */
+  private static Set<String> getStringOutputFunctionList() {
+    Set<String> set = new HashSet<>();
+    Method[] methods = StringFunctions.class.getDeclaredMethods();
+    for (Method method : methods) {
+      if (method.getReturnType() == String.class) {
+        set.add(method.getName().toUpperCase());
+      }
+    }
+    return set;

Review comment:
       Hmm, I see. Thanks for clarifying.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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

[GitHub] [pinot] amrishlal closed pull request #8253: Fix string comparisons.

Posted by GitBox <gi...@apache.org>.
amrishlal closed pull request #8253:
URL: https://github.com/apache/pinot/pull/8253


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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