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/09/03 19:35:09 UTC
[GitHub] [pinot] amrishlal opened a new pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
amrishlal opened a new pull request #7394:
URL: https://github.com/apache/pinot/pull/7394
## Description
Any query that compares two string columns will fail to execute. For example, the query
`SELECT * FROM myTable WHERE strColumn1=strColumn2`
throws NumberFormatException.
```
java.lang.NumberFormatException: For input string: "null"
at java.base/jdk.internal.math.FloatingDecimal.readJavaFormatString(FloatingDecimal.java:2054)
at java.base/jdk.internal.math.FloatingDecimal.parseDouble(FloatingDecimal.java:110)
at java.base/java.lang.Double.parseDouble(Double.java:543)
at org.apache.pinot.segment.local.segment.index.readers.StringDictionary.readDoubleValues(StringDictionary.java:105)
at org.apache.pinot.core.common.DataFetcher$ColumnValueReader.readDoubleValues(DataFetcher.java:403)
at org.apache.pinot.core.common.DataFetcher.fetchDoubleValues(DataFetcher.java:136)
at org.apache.pinot.core.common.DataBlockCache.getDoubleValuesForSVColumn(DataBlockCache.java:175)
at org.apache.pinot.core.operator.docvalsets.ProjectionBlockValSet.getDoubleValuesSV(ProjectionBlockValSet.java:89)
at org.apache.pinot.core.operator.transform.function.IdentifierTransformFunction.transformToDoubleValuesSV(IdentifierTransformFunction.java:94)
at org.apache.pinot.core.operator.transform.function.SubtractionTransformFunction.transformToDoubleValuesSV(SubtractionTransformFunction.java:89)
at org.apache.pinot.core.operator.dociditerators.ExpressionScanDocIdIterator.processProjectionBlock(ExpressionScanDocIdIterator.java:161)
at org.apache.pinot.core.operator.dociditerators.ExpressionScanDocIdIterator.next(ExpressionScanDocIdIterator.java:82)
at org.apache.pinot.core.operator.DocIdSetOperator.getNextBlock(DocIdSetOperator.java:69)
at org.apache.pinot.core.operator.DocIdSetOperator.getNextBlock(DocIdSetOperator.java:35)
```
This happens because `CalciteSqlParser.queryRewrite` will convert any expression of the form `column1 = column2` to `MINUS(column1, column2) = 0` and this doesn't work for `STRING` column type.
This PR fixes the issue when both the columns involved are `STRING` so that `MINUS(strColumn1, strColumn2) = 0` will get rewritten to `COMPARE(strColumn1, strColumn2) = 0`.
## 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] Jackie-Jiang merged pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7394:
URL: https://github.com/apache/pinot/pull/7394
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r704977393
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "COMPARE(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into COMPARE(strColumnAt some point, we should merge query rewrite phase with
+ * optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "COMPARE";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
Review comment:
Fixed.
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b42fca9) into [master](https://codecov.io/gh/apache/pinot/commit/8106b69a823fa525ca3450d747b0eb315ec0a67b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8106b69) will **decrease** coverage by `42.73%`.
> The diff coverage is `62.19%`.
> :exclamation: Current head b42fca9 differs from pull request most recent head 7b9372a. Consider uploading reports for the commit 7b9372a to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
=============================================
- Coverage 71.83% 29.10% -42.74%
+ Complexity 3350 85 -3265
=============================================
Files 1517 1509 -8
Lines 75003 74743 -260
Branches 10913 10906 -7
=============================================
- Hits 53879 21754 -32125
- Misses 17496 51018 +33522
+ Partials 3628 1971 -1657
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `29.10% <62.19%> (+0.04%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `5.35% <0.00%> (-65.56%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `83.33% <0.00%> (-16.67%)` | :arrow_down: |
| [...ry/aggregation/groupby/DefaultGroupByExecutor.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9ncm91cGJ5L0RlZmF1bHRHcm91cEJ5RXhlY3V0b3IuamF2YQ==) | `96.15% <0.00%> (-3.85%)` | :arrow_down: |
| [...a/org/apache/pinot/common/utils/PinotDataType.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvUGlub3REYXRhVHlwZS5qYXZh) | `28.64% <45.00%> (-49.93%)` | :arrow_down: |
| [...org/apache/pinot/core/data/table/TableResizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1RhYmxlUmVzaXplci5qYXZh) | `72.28% <64.47%> (-19.34%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `72.97% <72.97%> (ø)` | |
| [...tion/groupby/DictionaryBasedGroupKeyGenerator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9ncm91cGJ5L0RpY3Rpb25hcnlCYXNlZEdyb3VwS2V5R2VuZXJhdG9yLmphdmE=) | `43.34% <100.00%> (-46.25%)` | :arrow_down: |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `92.30% <100.00%> (-7.70%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| ... and [1057 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [8106b69...7b9372a](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r706501866
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
Review comment:
Fixed.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
Review comment:
Fixed.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
Review comment:
Moved to "default. Let's leave it as "switch" though as switch tends to perform better with a set of options as compared to if and probably easier to modify in future as well.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
Review comment:
Fixed.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ function.setOperator(COMPARE_OPERATOR_NAME);
+ }
+ }
+
+ /** @return true if expression is a column of numeric type. */
Review comment:
Fixed.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ function.setOperator(COMPARE_OPERATOR_NAME);
+ }
+ }
+
+ /** @return true if expression is a column of numeric type. */
+ private static boolean isStringColumn(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.IDENTIFIER) {
+ // Expression can not be a column.
Review comment:
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] sajjad-moradi commented on pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912830324
I think we should directly update `CalciteSqlParser.updateComparisonPredicate`. This has the following benefits:
1. CalciteSqlParser already rewrites queries to convert `WHERE a = b` to `WHERE a - b = 0`. Now we're adding one more optimizer to convert `WHERE a - b = 0` to `WHERE compare(a,b) = 0`. Instead we can directly convert `WHERE a = b` to `WHERE compare(a,b) = 0`.
2. Basically this is not an optimization, it's fixing a bug in the query rewrite logic.
3. `StringPredicateFilterOptimizer` code is very similar - not exact duplicate - to `CalciteSqlParser.updateComparisonPredicate`. With small change in the existing updateComparisonPredicate method, we can fix the bug.
4. We don't need to split the rewrite logic in two places. If something changes or a bug surfaces in the rewrite logic, we should only update one place in the 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] codecov-commenter edited a comment on pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2e2aa00) into [master](https://codecov.io/gh/apache/pinot/commit/8106b69a823fa525ca3450d747b0eb315ec0a67b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8106b69) will **decrease** coverage by `0.04%`.
> The diff coverage is `75.00%`.
> :exclamation: Current head 2e2aa00 differs from pull request most recent head a51cf23. Consider uploading reports for the commit a51cf23 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 65.02% 64.98% -0.05%
+ Complexity 3350 3347 -3
============================================
Files 1471 1472 +1
Lines 73078 73117 +39
Branches 10707 10715 +8
============================================
- Hits 47520 47514 -6
- Misses 22185 22216 +31
- Partials 3373 3387 +14
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `69.67% <75.00%> (-0.06%)` | :arrow_down: |
| unittests2 | `14.52% <0.00%> (-0.02%)` | :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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `75.67% <75.67%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `68.08% <0.00%> (-15.61%)` | :arrow_down: |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| [...e/pinot/core/transport/InstanceRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvSW5zdGFuY2VSZXF1ZXN0SGFuZGxlci5qYXZh) | `56.94% <0.00%> (-4.17%)` | :arrow_down: |
| [...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `74.61% <0.00%> (-3.63%)` | :arrow_down: |
| [.../aggregation/function/ModeAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9Nb2RlQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `86.52% <0.00%> (-1.89%)` | :arrow_down: |
| [.../core/operator/combine/GroupByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0dyb3VwQnlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `75.40% <0.00%> (-1.64%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [8106b69...a51cf23](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c9d706) into [master](https://codecov.io/gh/apache/pinot/commit/c27ae814c2d8772b4ebc27436cfa22b43d9c8f37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c27ae81) will **decrease** coverage by `42.80%`.
> The diff coverage is `82.92%`.
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
=============================================
- Coverage 71.89% 29.08% -42.81%
+ Complexity 3348 85 -3263
=============================================
Files 1517 1510 -7
Lines 75052 74789 -263
Branches 10936 10910 -26
=============================================
- Hits 53956 21752 -32204
- Misses 17475 51062 +33587
+ Partials 3621 1975 -1646
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `29.08% <82.92%> (+0.11%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `5.35% <0.00%> (-65.56%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `84.21% <84.21%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `92.30% <100.00%> (-7.70%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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/7394/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: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| ... and [1056 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [c27ae81...0c9d706](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ef2c470) into [master](https://codecov.io/gh/apache/pinot/commit/c27ae814c2d8772b4ebc27436cfa22b43d9c8f37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c27ae81) will **decrease** coverage by `0.84%`.
> The diff coverage is `80.00%`.
> :exclamation: Current head ef2c470 differs from pull request most recent head 3ee7813. Consider uploading reports for the commit 3ee7813 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 71.89% 71.04% -0.85%
Complexity 3348 3348
============================================
Files 1517 1518 +1
Lines 75052 75091 +39
Branches 10936 10944 +8
============================================
- Hits 53956 53351 -605
- Misses 17475 18093 +618
- Partials 3621 3647 +26
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.68% <72.50%> (+<0.01%)` | :arrow_up: |
| integration2 | `?` | |
| unittests1 | `69.71% <75.00%> (-0.04%)` | :arrow_down: |
| unittests2 | `14.52% <0.00%> (+<0.01%)` | :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/pinot/pull/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `81.08% <81.08%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...ore/operator/streaming/StreamingResponseUtils.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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: |
| [...ator/streaming/StreamingSelectionOnlyOperator.java](https://codecov.io/gh/apache/pinot/pull/7394/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%> (-92.31%)` | :arrow_down: |
| [...he/pinot/core/plan/StreamingSelectionPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7394/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%> (-90.00%)` | :arrow_down: |
| [...ller/api/access/BasicAuthAccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...ava/org/apache/pinot/core/auth/BasicAuthUtils.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9hdXRoL0Jhc2ljQXV0aFV0aWxzLmphdmE=) | `0.00% <0.00%> (-77.28%)` | :arrow_down: |
| ... and [83 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [c27ae81...3ee7813](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8c01fdc) into [master](https://codecov.io/gh/apache/pinot/commit/ffcf9b991431067c834bd4fb56fd7641c7fec172?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ffcf9b9) will **decrease** coverage by `2.20%`.
> The diff coverage is `80.48%`.
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 71.90% 69.70% -2.21%
+ Complexity 3433 3253 -180
============================================
Files 1518 1125 -393
Lines 75097 53046 -22051
Branches 10940 7997 -2943
============================================
- Hits 53999 36975 -17024
+ Misses 17469 13449 -4020
+ Partials 3629 2622 -1007
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.70% <80.48%> (-0.05%)` | :arrow_down: |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `81.57% <81.57%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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/7394/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/7394/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: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| ... and [603 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [ffcf9b9...8c01fdc](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2e2aa00) into [master](https://codecov.io/gh/apache/pinot/commit/8106b69a823fa525ca3450d747b0eb315ec0a67b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8106b69) will **decrease** coverage by `0.05%`.
> The diff coverage is `75.00%`.
> :exclamation: Current head 2e2aa00 differs from pull request most recent head a51cf23. Consider uploading reports for the commit a51cf23 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 69.72% 69.67% -0.06%
+ Complexity 3258 3255 -3
============================================
Files 1123 1124 +1
Lines 52912 52951 +39
Branches 7962 7970 +8
============================================
Hits 36894 36894
- Misses 13403 13429 +26
- Partials 2615 2628 +13
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `69.67% <75.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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `75.67% <75.67%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `68.08% <0.00%> (-15.61%)` | :arrow_down: |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| [...e/pinot/core/transport/InstanceRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvSW5zdGFuY2VSZXF1ZXN0SGFuZGxlci5qYXZh) | `56.94% <0.00%> (-4.17%)` | :arrow_down: |
| [.../aggregation/function/ModeAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9Nb2RlQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `86.52% <0.00%> (-1.89%)` | :arrow_down: |
| [.../core/operator/combine/GroupByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0dyb3VwQnlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `75.40% <0.00%> (-1.64%)` | :arrow_down: |
| [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/pinot/pull/7394/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.28% <0.00%> (-0.91%)` | :arrow_down: |
| ... and [4 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [8106b69...a51cf23](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r706855461
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,119 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String STRCMP_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ if (schema == null) {
+ return;
+ }
+
+ Expression filter = query.getFilterExpression();
+ if (filter != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with STRCMP if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ for (Expression operand : operands) {
+ optimizeExpression(operand, schema);
+ }
+ break;
+ }
+ default: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ 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;
+ }
+
+ String column = expression.getIdentifier().getName();
+ FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+ return fieldSpec == null ? false : fieldSpec.getDataType() == FieldSpec.DataType.STRING;
Review comment:
Fixed.
--
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] mayankshriv removed a comment on pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
mayankshriv removed a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912896211
Is this a backward incompatible change? What happens to use cases that are using the `MINUS` transform function? Perhaps better to create a separate new one as opposed to renaming the existing one and causing backward incompatibility?
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eba9726) into [master](https://codecov.io/gh/apache/pinot/commit/c27ae814c2d8772b4ebc27436cfa22b43d9c8f37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c27ae81) will **decrease** coverage by `6.93%`.
> The diff coverage is `80.48%`.
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 71.89% 64.95% -6.94%
+ Complexity 3348 3344 -4
============================================
Files 1517 1473 -44
Lines 75052 73212 -1840
Branches 10936 10742 -194
============================================
- Hits 53956 47556 -6400
- Misses 17475 22255 +4780
+ Partials 3621 3401 -220
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.61% <80.48%> (-0.14%)` | :arrow_down: |
| unittests2 | `14.51% <0.00%> (+<0.01%)` | :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/pinot/pull/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `81.57% <81.57%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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/7394/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/7394/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: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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: |
| ... and [362 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [c27ae81...eba9726](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8c01fdc) into [master](https://codecov.io/gh/apache/pinot/commit/ffcf9b991431067c834bd4fb56fd7641c7fec172?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ffcf9b9) will **decrease** coverage by `9.66%`.
> The diff coverage is `90.24%`.
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 71.90% 62.23% -9.67%
+ Complexity 3433 3338 -95
============================================
Files 1518 1510 -8
Lines 75097 74789 -308
Branches 10940 10910 -30
============================================
- Hits 53999 46547 -7452
- Misses 17469 24878 +7409
+ Partials 3629 3364 -265
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `28.97% <82.92%> (-0.12%)` | :arrow_down: |
| unittests1 | `69.70% <80.48%> (-0.05%)` | :arrow_down: |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `92.10% <92.10%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...pinot/controller/recommender/io/ConfigManager.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9Db25maWdNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...troller/recommender/io/metadata/FieldMetadata.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9tZXRhZGF0YS9GaWVsZE1ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...roller/recommender/rules/impl/BloomFilterRule.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL0Jsb29tRmlsdGVyUnVsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ler/recommender/data/generator/BytesGenerator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9kYXRhL2dlbmVyYXRvci9CeXRlc0dlbmVyYXRvci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...er/recommender/io/metadata/SchemaWithMetaData.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9tZXRhZGF0YS9TY2hlbWFXaXRoTWV0YURhdGEuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...r/recommender/rules/impl/AggregateMetricsRule.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL0FnZ3JlZ2F0ZU1ldHJpY3NSdWxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [281 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [ffcf9b9...8c01fdc](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r704976661
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -389,4 +389,15 @@ public static int hammingDistance(String input1, String input2) {
public static boolean contains(String input, String substring) {
return input.contains(substring);
}
+
+ /**
+ * Compare input strings lexicographically.
+ * @return the value 0 if the first string argument is equal to second string; a value less than 0 if first string
+ * argument is lexicographically less than the second string argument; and a value greater than 0 if the first string
+ * argument is lexicographically greater than the second string argument.
+ */
+ @ScalarFunction
+ public static int compare(String input1, String input2) {
Review comment:
Renamed.
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ad80ca3) into [master](https://codecov.io/gh/apache/pinot/commit/c27ae814c2d8772b4ebc27436cfa22b43d9c8f37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c27ae81) will **decrease** coverage by `39.58%`.
> The diff coverage is `57.44%`.
> :exclamation: Current head ad80ca3 differs from pull request most recent head eba9726. Consider uploading reports for the commit eba9726 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
=============================================
- Coverage 71.89% 32.30% -39.59%
+ Complexity 3348 93 -3255
=============================================
Files 1517 1510 -7
Lines 75052 74789 -263
Branches 10936 10910 -26
=============================================
- Hits 53956 24160 -29796
- Misses 17475 48560 +31085
+ Partials 3621 2069 -1552
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.61% <38.29%> (-0.06%)` | :arrow_down: |
| integration2 | `29.14% <57.44%> (+0.17%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `5.35% <0.00%> (-65.56%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/env/PinotConfiguration.java](https://codecov.io/gh/apache/pinot/pull/7394/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%> (-97.44%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/Obfuscator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvT2JmdXNjYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...he/pinot/common/function/scalar/JsonFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0pzb25GdW5jdGlvbnMuamF2YQ==) | `30.50% <66.66%> (-50.20%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `84.21% <84.21%> (ø)` | |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/pinot/pull/7394/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==) | `94.73% <94.11%> (-5.27%)` | :arrow_down: |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `92.30% <100.00%> (-7.70%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| ... and [994 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [c27ae81...eba9726](https://codecov.io/gh/apache/pinot/pull/7394?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] Jackie-Jiang commented on a change in pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r703817541
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -389,4 +389,15 @@ public static int hammingDistance(String input1, String input2) {
public static boolean contains(String input, String substring) {
return input.contains(substring);
}
+
+ /**
+ * Compare input strings lexicographically.
+ * @return the value 0 if the first string argument is equal to second string; a value less than 0 if first string
+ * argument is lexicographically less than the second string argument; and a value greater than 0 if the first string
+ * argument is lexicographically greater than the second string argument.
+ */
+ @ScalarFunction
+ public static int compare(String input1, String input2) {
Review comment:
Let's rename it to `strcmp` which is a standard MySQL function. Since we don't have type matching yet, we want to differentiate the string comparison from comparison of other types
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "COMPARE(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into COMPARE(strColumnAt some point, we should merge query rewrite phase with
+ * optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "COMPARE";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
Review comment:
Let's check if the schema is null here before calling `optimizeExpression` and remove the null checks there
--
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] Jackie-Jiang commented on a change in pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r705787714
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
Review comment:
This should be a for loop
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
Review comment:
```suggestion
/** Traverse an expression tree to replace MINUS function with STRCMP if function operands are STRING. */
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
Review comment:
To simplify, we can use if
```suggestion
if (kind == AND || kind == OR) {
for ...
} else {
replaceMinusWithCompareForStrings(operands.get(0), schema);
}
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ function.setOperator(COMPARE_OPERATOR_NAME);
+ }
+ }
+
+ /** @return true if expression is a column of numeric type. */
+ private static boolean isStringColumn(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.IDENTIFIER) {
+ // Expression can not be a column.
+ return false;
+ }
+
+ String column = expression.getIdentifier().getName();
+ FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+ if (fieldSpec == null || !fieldSpec.isSingleValueField()) {
+ // Expression can not be a column name.
+ return false;
+ }
+
+ return schema.getFieldSpecFor(column).getDataType() == FieldSpec.DataType.STRING;
Review comment:
I don't think we need to check single-value. IIRC, we will pass the first element of the MV to the function
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ function.setOperator(COMPARE_OPERATOR_NAME);
+ }
+ }
+
+ /** @return true if expression is a column of numeric type. */
+ private static boolean isStringColumn(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.IDENTIFIER) {
+ // Expression can not be a column.
Review comment:
```suggestion
// Expression is not a column.
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ function.setOperator(COMPARE_OPERATOR_NAME);
+ }
+ }
+
+ /** @return true if expression is a column of numeric type. */
Review comment:
```suggestion
/** @return true if expression is a column of string type. */
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
Review comment:
We should put this in the default to cover all cases. E.g. when user explicitly put (str1 - str2) instead of converted from the parser
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "COMPARE(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into COMPARE(strColumnAt some point, we should merge query rewrite phase with
+ * optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "COMPARE";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
Review comment:
(nit) I feel it will be cleaner if we do
```suggestion
if (schema == null) {
return;
}
...
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ function.setOperator(COMPARE_OPERATOR_NAME);
+ }
+ }
+
+ /** @return true if expression is a column of numeric type. */
+ private static boolean isStringColumn(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.IDENTIFIER) {
+ // Expression can not be a column.
+ return false;
+ }
+
+ String column = expression.getIdentifier().getName();
+ FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+ if (fieldSpec == null || !fieldSpec.isSingleValueField()) {
+ // Expression can not be a column name.
+ return false;
+ }
+
+ return schema.getFieldSpecFor(column).getDataType() == FieldSpec.DataType.STRING;
Review comment:
Avoid fetching field spec twice
```suggestion
return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
Review comment:
```suggestion
private static final String STRCMP_OPERATOR_NAME = "STRCMP";
```
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7b9372a) into [master](https://codecov.io/gh/apache/pinot/commit/8106b69a823fa525ca3450d747b0eb315ec0a67b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8106b69) will **decrease** coverage by `2.12%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 71.83% 69.71% -2.13%
+ Complexity 3350 3255 -95
============================================
Files 1517 1124 -393
Lines 75003 53000 -22003
Branches 10913 7993 -2920
============================================
- Hits 53879 36949 -16930
+ Misses 17496 13425 -4071
+ Partials 3628 2626 -1002
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.71% <75.00%> (-0.02%)` | :arrow_down: |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `75.67% <75.67%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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/7394/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/7394/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: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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: |
| ... and [599 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [8106b69...7b9372a](https://codecov.io/gh/apache/pinot/pull/7394?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 pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912768714
@siddharthteotia 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.
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r706505059
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "COMPARE(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into COMPARE(strColumnAt some point, we should merge query rewrite phase with
+ * optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "COMPARE";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
Review comment:
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2e2aa00) into [master](https://codecov.io/gh/apache/pinot/commit/8106b69a823fa525ca3450d747b0eb315ec0a67b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8106b69) will **decrease** coverage by `0.00%`.
> The diff coverage is `80.00%`.
> :exclamation: Current head 2e2aa00 differs from pull request most recent head a51cf23. Consider uploading reports for the commit a51cf23 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 70.61% 70.61% -0.01%
+ Complexity 3350 3347 -3
============================================
Files 1517 1518 +1
Lines 75003 75042 +39
Branches 10913 10921 +8
============================================
+ Hits 52962 52988 +26
- Misses 18415 18417 +2
- Partials 3626 3637 +11
```
| Flag | Coverage Δ | |
|---|---|---|
| integration2 | `29.12% <75.00%> (+0.06%)` | :arrow_up: |
| unittests1 | `69.67% <75.00%> (-0.06%)` | :arrow_down: |
| unittests2 | `14.52% <0.00%> (-0.02%)` | :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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `81.08% <81.08%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `68.79% <0.00%> (-15.61%)` | :arrow_down: |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| [...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh) | `78.02% <0.00%> (-2.20%)` | :arrow_down: |
| [.../aggregation/function/ModeAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9Nb2RlQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `86.52% <0.00%> (-1.89%)` | :arrow_down: |
| [...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `80.82% <0.00%> (-1.04%)` | :arrow_down: |
| [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/pinot/pull/7394/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.28% <0.00%> (-0.91%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [8106b69...a51cf23](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r704977257
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -389,4 +389,15 @@ public static int hammingDistance(String input1, String input2) {
public static boolean contains(String input, String substring) {
return input.contains(substring);
}
+
+ /**
+ * Compare input strings lexicographically.
+ * @return the value 0 if the first string argument is equal to second string; a value less than 0 if first string
+ * argument is lexicographically less than the second string argument; and a value greater than 0 if the first string
+ * argument is lexicographically greater than the second string argument.
+ */
+ @ScalarFunction
+ public static int compare(String input1, String input2) {
Review comment:
Renamed.
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3c56082) into [master](https://codecov.io/gh/apache/pinot/commit/ffcf9b991431067c834bd4fb56fd7641c7fec172?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ffcf9b9) will **decrease** coverage by `57.39%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 3c56082 differs from pull request most recent head 8c01fdc. Consider uploading reports for the commit 8c01fdc to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
=============================================
- Coverage 71.90% 14.50% -57.40%
+ Complexity 3433 92 -3341
=============================================
Files 1518 1473 -45
Lines 75097 73212 -1885
Branches 10940 10742 -198
=============================================
- Hits 53999 10623 -43376
- Misses 17469 61829 +44360
+ Partials 3629 760 -2869
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.50% <0.00%> (-0.02%)` | :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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `0.00% <0.00%> (-70.91%)` | :arrow_down: |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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%> (ø)` | |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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/7394/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/7394/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/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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: |
| ... and [1200 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [ffcf9b9...8c01fdc](https://codecov.io/gh/apache/pinot/pull/7394?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 pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912864460
> CalciteSqlParser already rewrites queries to convert WHERE a = b to WHERE a - b = 0. Now we're adding one more optimizer to convert WHERE a - b = 0 to WHERE compare(a,b) = 0. Instead we can directly convert WHERE a = b to WHERE compare(a,b) = 0
>StringPredicateFilterOptimizer code is very similar - not exact duplicate - to CalciteSqlParser.updateComparisonPredicate. With small change in the existing updateComparisonPredicate method, we can fix the bug.
`CalciteSqlParser` doesn't know whether columns `a` and `b` are `STRING`, `INTEGER`, or `BYTES`, or any other datatype, so we can't really rewrite `WHERE a = b` into `WHERE COMPARE(a,b) = 0` when `a` and `b` are `STRING` and to `WHERE MINUS(a,b)=0` when `a` and `b` are numbers. Also, we cannot overload `MINUS` function to work on `STRING` since `ScalarFunction` currently cannot be overloaded (there are several issues already open on this as this has implications on query correctness and type handling but I don't think there is a good solution yet).
> Basically this is not an optimization, it's fixing a bug in the query rewrite logic.
True, but we also need to keep in mind that `QueryOptimizer` isn't really an optimizer, its a rewriter :-)
> We don't need to split the rewrite logic in two places. If something changes or a bug surfaces in the rewrite logic, we should only update one place in the code.
The rewrite logic is already split into two places (one in `CalciteSqlParser` and other in `QueryOptimizer`). So given the issues, this serves as a good "fix" until we do some major refactoring like 1) using Schema and TableConfig information in CalciteSqlParser in which case it should just be merged with `QueryOptimizer` since QueryOptimizer does that already, or 2) fixing the function overloading and type resolution issues with ScalarFunction). I added some comment about this in `StringPredicateFilterOptimizer` and can add more details if needed?
--
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] mayankshriv commented on pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912896211
Is this a backward incompatible change? What happens to use cases that are using the `MINUS` transform function? Perhaps better to create a separate new one as opposed to renaming the existing one and causing backward incompatibility?
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eba9726) into [master](https://codecov.io/gh/apache/pinot/commit/c27ae814c2d8772b4ebc27436cfa22b43d9c8f37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c27ae81) will **decrease** coverage by `2.27%`.
> The diff coverage is `80.48%`.
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
============================================
- Coverage 71.89% 69.61% -2.28%
+ Complexity 3348 3252 -96
============================================
Files 1517 1125 -392
Lines 75052 53046 -22006
Branches 10936 7997 -2939
============================================
- Hits 53956 36927 -17029
+ Misses 17475 13476 -3999
+ Partials 3621 2643 -978
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.61% <80.48%> (-0.14%)` | :arrow_down: |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `81.57% <81.57%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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/7394/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/7394/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: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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: |
| ... and [603 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [c27ae81...eba9726](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r704976833
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "COMPARE(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into COMPARE(strColumnAt some point, we should merge query rewrite phase with
+ * optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "COMPARE";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
Review comment:
Fixed.
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ef2c470) into [master](https://codecov.io/gh/apache/pinot/commit/c27ae814c2d8772b4ebc27436cfa22b43d9c8f37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c27ae81) will **increase** coverage by `0.00%`.
> The diff coverage is `80.00%`.
> :exclamation: Current head ef2c470 differs from pull request most recent head 3ee7813. Consider uploading reports for the commit 3ee7813 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
=========================================
Coverage 71.89% 71.89%
Complexity 3348 3348
=========================================
Files 1517 1518 +1
Lines 75052 75091 +39
Branches 10936 10944 +8
=========================================
+ Hits 53956 53986 +30
+ Misses 17475 17459 -16
- Partials 3621 3646 +25
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.68% <72.50%> (+<0.01%)` | :arrow_up: |
| integration2 | `29.05% <72.50%> (+0.08%)` | :arrow_up: |
| unittests1 | `69.71% <75.00%> (-0.04%)` | :arrow_down: |
| unittests2 | `14.52% <0.00%> (+<0.01%)` | :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/pinot/pull/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `69.64% <0.00%> (-1.27%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `81.08% <81.08%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `86.52% <0.00%> (-7.81%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| [...ntroller/helix/core/minion/TaskMetricsEmitter.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9UYXNrTWV0cmljc0VtaXR0ZXIuamF2YQ==) | `86.36% <0.00%> (-4.55%)` | :arrow_down: |
| [.../pinot/core/query/scheduler/PriorityScheduler.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvUHJpb3JpdHlTY2hlZHVsZXIuamF2YQ==) | `80.82% <0.00%> (-2.74%)` | :arrow_down: |
| [...core/query/pruner/SelectionQuerySegmentPruner.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvU2VsZWN0aW9uUXVlcnlTZWdtZW50UHJ1bmVyLmphdmE=) | `86.36% <0.00%> (-2.28%)` | :arrow_down: |
| [...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh) | `78.02% <0.00%> (-2.20%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [c27ae81...3ee7813](https://codecov.io/gh/apache/pinot/pull/7394?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] Jackie-Jiang commented on a change in pull request #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r706519095
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,119 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String STRCMP_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ if (schema == null) {
+ return;
+ }
+
+ Expression filter = query.getFilterExpression();
+ if (filter != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with STRCMP if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ for (Expression operand : operands) {
+ optimizeExpression(operand, schema);
+ }
+ break;
+ }
+ default: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ 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;
+ }
+
+ String column = expression.getIdentifier().getName();
+ FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+ return fieldSpec == null ? false : fieldSpec.getDataType() == FieldSpec.DataType.STRING;
Review comment:
```suggestion
return fieldSpec != null && fieldSpec.getDataType() == FieldSpec.DataType.STRING;
```
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#discussion_r706502092
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -0,0 +1,127 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.optimizer.statement;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.request.Expression;
+import org.apache.pinot.common.request.ExpressionType;
+import org.apache.pinot.common.request.Function;
+import org.apache.pinot.common.request.PinotQuery;
+import org.apache.pinot.pql.parsers.pql2.ast.FilterKind;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+
+
+/**
+ * Given two column names 'strColumn1' and 'strColumn1', CalciteSqlParser.queryRewrite will turn WHERE and HAVING
+ * expressions of form "strColumn1 <operator> strColumn2" into "MINUS(strColumn1,strColumn2) <operator> 0" regardless
+ * of the column datatype. The resulting query will fail to evaluate since the MINUS operator does not work with the
+ * STRING column type. This class rewrites expressions of form "MINUS(strColumn1,strColumn2) <operator> 0" to
+ * "STRCMP(strColumn1, strColumn2) <operator> 0" to fix the issue.
+ *
+ * Currently, rewrite phase (see CalciteSqlParser.queryRewrite) does not have access to schema; hence, we need to again
+ * rewrite MINUS(strColumn1, strColumn2) into STRCMP(strColumn1, strColumn2). At some point, we should merge query
+ * rewrite phase with optimizer phase to avoid such issues altogether.
+ */
+public class StringPredicateFilterOptimizer implements StatementOptimizer {
+ private static final String MINUS_OPERATOR_NAME = "MINUS";
+ private static final String COMPARE_OPERATOR_NAME = "STRCMP";
+
+ @Override
+ public void optimize(PinotQuery query, @Nullable TableConfig tableConfig, @Nullable Schema schema) {
+ Expression filter = query.getFilterExpression();
+ if (filter != null && schema != null) {
+ optimizeExpression(filter, schema);
+ }
+
+ Expression expression = query.getHavingExpression();
+ if (expression != null && schema != null) {
+ optimizeExpression(expression, schema);
+ }
+ }
+
+ /** Traverse an expression tree to replace MINUS function with COMPARE if function operands are STRING. */
+ private static void optimizeExpression(Expression expression, Schema schema) {
+ ExpressionType type = expression.getType();
+ if (type != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ Function function = expression.getFunctionCall();
+ String operator = function.getOperator();
+ List<Expression> operands = function.getOperands();
+ FilterKind kind = FilterKind.valueOf(operator);
+ switch (kind) {
+ case AND:
+ case OR: {
+ optimizeExpression(operands.get(0), schema);
+ break;
+ }
+ case EQUALS:
+ case NOT_EQUALS:
+ case GREATER_THAN:
+ case GREATER_THAN_OR_EQUAL:
+ case LESS_THAN:
+ case LESS_THAN_OR_EQUAL:
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ replaceMinusWithCompareForStrings(operands.get(0), schema);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
+ /** Replace the operator of a MINUS function with COMPARE if both operands are STRING. */
+ private static void replaceMinusWithCompareForStrings(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.FUNCTION) {
+ // We have nothing to rewrite if expression is not a function.
+ return;
+ }
+
+ 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)) {
+ function.setOperator(COMPARE_OPERATOR_NAME);
+ }
+ }
+
+ /** @return true if expression is a column of numeric type. */
+ private static boolean isStringColumn(Expression expression, Schema schema) {
+ if (expression.getType() != ExpressionType.IDENTIFIER) {
+ // Expression can not be a column.
+ return false;
+ }
+
+ String column = expression.getIdentifier().getName();
+ FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+ if (fieldSpec == null || !fieldSpec.isSingleValueField()) {
+ // Expression can not be a column name.
+ return false;
+ }
+
+ return schema.getFieldSpecFor(column).getDataType() == FieldSpec.DataType.STRING;
Review comment:
Refactored code a bit.
--
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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ad80ca3) into [master](https://codecov.io/gh/apache/pinot/commit/c27ae814c2d8772b4ebc27436cfa22b43d9c8f37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c27ae81) will **decrease** coverage by `41.27%`.
> The diff coverage is `38.29%`.
> :exclamation: Current head ad80ca3 differs from pull request most recent head eba9726. Consider uploading reports for the commit eba9726 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
=============================================
- Coverage 71.89% 30.61% -41.28%
+ Complexity 3348 88 -3260
=============================================
Files 1517 1510 -7
Lines 75052 74789 -263
Branches 10936 10910 -26
=============================================
- Hits 53956 22897 -31059
- Misses 17475 49871 +32396
+ Partials 3621 2021 -1600
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.61% <38.29%> (-0.06%)` | :arrow_down: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `0.00% <0.00%> (-70.91%)` | :arrow_down: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...a/org/apache/pinot/spi/env/PinotConfiguration.java](https://codecov.io/gh/apache/pinot/pull/7394/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%> (-97.44%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/Obfuscator.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvT2JmdXNjYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...he/pinot/common/function/scalar/JsonFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0pzb25GdW5jdGlvbnMuamF2YQ==) | `22.03% <33.33%> (-58.67%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `84.21% <84.21%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `92.30% <100.00%> (-7.70%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| ... and [1042 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [c27ae81...eba9726](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394: Replace MINUS(strColum1, strColumn2) with COMPARE(strColumn1,strColumn2)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7394:
URL: https://github.com/apache/pinot/pull/7394#issuecomment-912821972
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7394?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 [#7394](https://codecov.io/gh/apache/pinot/pull/7394?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c9d706) into [master](https://codecov.io/gh/apache/pinot/commit/c27ae814c2d8772b4ebc27436cfa22b43d9c8f37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c27ae81) will **decrease** coverage by `39.63%`.
> The diff coverage is `82.92%`.
[](https://codecov.io/gh/apache/pinot/pull/7394?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 #7394 +/- ##
=============================================
- Coverage 71.89% 32.26% -39.64%
+ Complexity 3348 93 -3255
=============================================
Files 1517 1510 -7
Lines 75052 74789 -263
Branches 10936 10910 -26
=============================================
- Hits 53956 24128 -29828
- Misses 17475 48591 +31116
+ Partials 3621 2070 -1551
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.42% <82.92%> (-0.25%)` | :arrow_down: |
| integration2 | `29.08% <82.92%> (+0.11%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
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/7394?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/scalar/StringFunctions.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL1N0cmluZ0Z1bmN0aW9ucy5qYXZh) | `5.35% <0.00%> (-65.56%)` | :arrow_down: |
| [...izer/statement/StringPredicateFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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) | `84.21% <84.21%> (ø)` | |
| [...che/pinot/core/query/optimizer/QueryOptimizer.java](https://codecov.io/gh/apache/pinot/pull/7394/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvUXVlcnlPcHRpbWl6ZXIuamF2YQ==) | `92.30% <100.00%> (-7.70%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7394/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: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7394/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/7394/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/7394/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: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7394/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%)` | :arrow_down: |
| ... and [999 more](https://codecov.io/gh/apache/pinot/pull/7394/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/7394?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/7394?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 [c27ae81...0c9d706](https://codecov.io/gh/apache/pinot/pull/7394?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