You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/02/05 20:36:53 UTC
[GitHub] [pinot] richardstartin opened a new pull request #8137: do not identify function types by throwing exceptions
richardstartin opened a new pull request #8137:
URL: https://github.com/apache/pinot/pull/8137
This PR allows function (transform/aggregation) type to be identified without throwing exceptions when an expression (e.g. "ASC") is not a function type, this reduces allocation rate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r801093976
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
##########
@@ -81,14 +87,28 @@ public String getName() {
return _name;
}
+ public static boolean isAggregationFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
+ try {
+ getAggregationFunctionType(functionName);
+ return true;
+ } catch (Exception ignore) {
+ }
+ }
+ String upperCaseFunctionName = functionName.replace("_", "").toUpperCase();
+ return NAMES.contains(upperCaseFunctionName);
+ }
+
/**
* Returns the corresponding aggregation function type for the given function name.
* <p>NOTE: Underscores in the function name are ignored.
*/
public static AggregationFunctionType getAggregationFunctionType(String functionName) {
- String upperCaseFunctionName = StringUtils.remove(functionName, '_').toUpperCase();
- if (upperCaseFunctionName.startsWith("PERCENTILE")) {
- String remainingFunctionName = upperCaseFunctionName.substring(10);
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
Review comment:
This is case insensitive without allocation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1032050624
The test failure should be addressed by #8154
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin merged pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin merged pull request #8137:
URL: https://github.com/apache/pinot/pull/8137
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1032022397
@Jackie-Jiang here are some numbers for the query I was tuning (on my MacBook so incomparable to numbers on different PRs)
master
```
Benchmark (_intBaseValue) (_numRows) Mode Cnt Score Error Units
BenchmarkFilteredAggregations.testNonFilteredAggregations 0 1500000 avgt 5 32506.601 ± 4192.330 us/op
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.alloc.rate 0 1500000 avgt 5 22.952 ± 2.747 MB/sec
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.alloc.rate.norm 0 1500000 avgt 5 1171670.000 ± 15616.821 B/op
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.churn.G1_Eden_Space 0 1500000 avgt 5 37.526 ± 323.107 MB/sec
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.churn.G1_Eden_Space.norm 0 1500000 avgt 5 1914496.826 ± 16484395.047 B/op
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.churn.G1_Old_Gen 0 1500000 avgt 5 7.749 ± 66.725 MB/sec
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.churn.G1_Old_Gen.norm 0 1500000 avgt 5 395366.090 ± 3404221.273 B/op
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.count 0 1500000 avgt 5 1.000 counts
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.time 0 1500000 avgt 5 13.000 ms
```
branch
```
Benchmark (_intBaseValue) (_numRows) Mode Cnt Score Error Units
BenchmarkFilteredAggregations.testNonFilteredAggregations 0 1500000 avgt 5 28881.564 ± 2888.988 us/op
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.alloc.rate 0 1500000 avgt 5 20.376 ± 1.846 MB/sec
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.alloc.rate.norm 0 1500000 avgt 5 922164.440 ± 30175.150 B/op
BenchmarkFilteredAggregations.testNonFilteredAggregations:·gc.count 0 1500000 avgt 5 ≈ 0 counts
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin edited a comment on pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin edited a comment on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1031973175
> Agree on avoiding the exception in normal execution path. Do we have some performance number back this change?
I had numbers for a few of these PRs combined on #8134 (which have been submitted separately because some users prefer tightly scoped changes) and this was just an incremental improvement. I can measure this separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r801114043
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -101,12 +107,28 @@
// Geo indexing
GEOTOH3("geoToH3");
+ private static final Set<String> NAMES = Arrays.stream(values())
+ .flatMap(func -> Stream.of(func.getName(), func.getName().replace("_", "").toUpperCase(),
+ func.getName().toUpperCase(), func.getName().toLowerCase(), func.name(), func.name().toLowerCase()))
+ .collect(Collectors.toSet());
+
private final String _name;
TransformFunctionType(String name) {
_name = name;
}
+ public static boolean isTransformFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ // scalar functions
+ if (FunctionRegistry.containsFunction(functionName)) {
+ return true;
+ }
+ return NAMES.contains(functionName.toUpperCase().replace("_", ""));
Review comment:
It looks like `StringUtils.remove` is quite a lot faster on JDK11 when there are characters to remove, but allocates a little bit more, I will change this back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r801093406
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -101,12 +107,28 @@
// Geo indexing
GEOTOH3("geoToH3");
+ private static final Set<String> NAMES = Arrays.stream(values())
+ .flatMap(func -> Stream.of(func.getName(), func.getName().replace("_", "").toUpperCase(),
+ func.getName().toUpperCase(), func.getName().toLowerCase(), func.name(), func.name().toLowerCase()))
+ .collect(Collectors.toSet());
+
private final String _name;
TransformFunctionType(String name) {
_name = name;
}
+ public static boolean isTransformFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ // scalar functions
+ if (FunctionRegistry.containsFunction(functionName)) {
+ return true;
+ }
+ return NAMES.contains(functionName.toUpperCase().replace("_", ""));
Review comment:
I honestly have no idea, but it's very hard to detect fast/slow code by eye.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r801110383
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
##########
@@ -81,14 +87,28 @@ public String getName() {
return _name;
}
+ public static boolean isAggregationFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
+ try {
+ getAggregationFunctionType(functionName);
+ return true;
+ } catch (Exception ignore) {
+ }
+ }
+ String upperCaseFunctionName = functionName.replace("_", "").toUpperCase();
+ return NAMES.contains(upperCaseFunctionName);
+ }
+
/**
* Returns the corresponding aggregation function type for the given function name.
* <p>NOTE: Underscores in the function name are ignored.
*/
public static AggregationFunctionType getAggregationFunctionType(String functionName) {
- String upperCaseFunctionName = StringUtils.remove(functionName, '_').toUpperCase();
- if (upperCaseFunctionName.startsWith("PERCENTILE")) {
- String remainingFunctionName = upperCaseFunctionName.substring(10);
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
Review comment:
`regionMatches` feels like the better option to me:
```
Benchmark (_function) Mode Cnt Score Error Units
StartsWith.regionMatches percentile_foo avgt 5 14.793 ± 0.467 ns/op
StartsWith.regionMatches:·gc.alloc.rate percentile_foo avgt 5 ≈ 10⁻⁴ MB/sec
StartsWith.regionMatches:·gc.alloc.rate.norm percentile_foo avgt 5 ≈ 10⁻⁵ B/op
StartsWith.regionMatches:·gc.count percentile_foo avgt 5 ≈ 0 counts
StartsWith.regionMatches PERCENTILE_FOO avgt 5 26.785 ± 0.480 ns/op
StartsWith.regionMatches:·gc.alloc.rate PERCENTILE_FOO avgt 5 ≈ 10⁻⁴ MB/sec
StartsWith.regionMatches:·gc.alloc.rate.norm PERCENTILE_FOO avgt 5 ≈ 10⁻⁵ B/op
StartsWith.regionMatches:·gc.count PERCENTILE_FOO avgt 5 ≈ 0 counts
StartsWith.regionMatches doesn't match avgt 5 7.091 ± 0.034 ns/op
StartsWith.regionMatches:·gc.alloc.rate doesn't match avgt 5 ≈ 10⁻⁴ MB/sec
StartsWith.regionMatches:·gc.alloc.rate.norm doesn't match avgt 5 ≈ 10⁻⁶ B/op
StartsWith.regionMatches:·gc.count doesn't match avgt 5 ≈ 0 counts
StartsWith.startsWith percentile_foo avgt 5 36.754 ± 0.447 ns/op
StartsWith.startsWith:·gc.alloc.rate percentile_foo avgt 5 965.883 ± 11.124 MB/sec
StartsWith.startsWith:·gc.alloc.rate.norm percentile_foo avgt 5 56.000 ± 0.001 B/op
StartsWith.startsWith:·gc.churn.G1_Eden_Space percentile_foo avgt 5 970.448 ± 4.635 MB/sec
StartsWith.startsWith:·gc.churn.G1_Eden_Space.norm percentile_foo avgt 5 56.265 ± 0.591 B/op
StartsWith.startsWith:·gc.churn.G1_Old_Gen percentile_foo avgt 5 0.001 ± 0.005 MB/sec
StartsWith.startsWith:·gc.churn.G1_Old_Gen.norm percentile_foo avgt 5 ≈ 10⁻⁴ B/op
StartsWith.startsWith:·gc.count percentile_foo avgt 5 40.000 counts
StartsWith.startsWith:·gc.time percentile_foo avgt 5 22.000 ms
StartsWith.startsWith PERCENTILE_FOO avgt 5 16.395 ± 0.523 ns/op
StartsWith.startsWith:·gc.alloc.rate PERCENTILE_FOO avgt 5 ≈ 10⁻⁴ MB/sec
StartsWith.startsWith:·gc.alloc.rate.norm PERCENTILE_FOO avgt 5 ≈ 10⁻⁵ B/op
StartsWith.startsWith:·gc.count PERCENTILE_FOO avgt 5 ≈ 0 counts
StartsWith.startsWith doesn't match avgt 5 31.228 ± 1.392 ns/op
StartsWith.startsWith:·gc.alloc.rate doesn't match avgt 5 1137.484 ± 48.866 MB/sec
StartsWith.startsWith:·gc.alloc.rate.norm doesn't match avgt 5 56.000 ± 0.001 B/op
StartsWith.startsWith:·gc.churn.G1_Eden_Space doesn't match avgt 5 1139.195 ± 256.337 MB/sec
StartsWith.startsWith:·gc.churn.G1_Eden_Space.norm doesn't match avgt 5 56.079 ± 11.876 B/op
StartsWith.startsWith:·gc.churn.G1_Old_Gen doesn't match avgt 5 0.002 ± 0.005 MB/sec
StartsWith.startsWith:·gc.churn.G1_Old_Gen.norm doesn't match avgt 5 ≈ 10⁻⁴ B/op
StartsWith.startsWith:·gc.count doesn't match avgt 5 47.000 counts
StartsWith.startsWith:·gc.time doesn't match avgt 5 25.000 ms
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1031973175
> Agree on avoiding the exception in normal execution path. Do we have some performance number back this change?
I had numbers for a few of these PRs (which have been submitted separately because some users prefer tightly scoped changes) and this was just an incremental improvement. I can measure this separately.
--
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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1030699838
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8137?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 [#8137](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f12498d) into [master](https://codecov.io/gh/apache/pinot/commit/a47af499ddb3b878a0bd0c62264c8bcf1cbd1f20?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a47af49) will **increase** coverage by `34.01%`.
> The diff coverage is `93.33%`.
[](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137 +/- ##
=============================================
+ Coverage 30.69% 64.71% +34.01%
- Complexity 0 4303 +4303
=============================================
Files 1613 1578 -35
Lines 83952 82432 -1520
Branches 12597 12433 -164
=============================================
+ Hits 25768 53342 +27574
+ Misses 55889 25273 -30616
- Partials 2295 3817 +1522
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.89% <93.33%> (?)` | |
| unittests2 | `14.18% <0.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/pinot/common/utils/SqlResultComparator.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU3FsUmVzdWx0Q29tcGFyYXRvci5qYXZh) | `0.00% <0.00%> (-18.85%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `91.35% <92.30%> (+91.35%)` | :arrow_up: |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (+10.95%)` | :arrow_up: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `73.36% <100.00%> (+19.09%)` | :arrow_up: |
| [...pql/parsers/PinotQuery2BrokerRequestConverter.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QaW5vdFF1ZXJ5MkJyb2tlclJlcXVlc3RDb252ZXJ0ZXIuamF2YQ==) | `88.51% <100.00%> (+4.72%)` | :arrow_up: |
| [...inot/pql/parsers/pql2/ast/OutputColumnAstNode.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9PdXRwdXRDb2x1bW5Bc3ROb2RlLmphdmE=) | `95.12% <100.00%> (+36.58%)` | :arrow_up: |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `86.95% <100.00%> (+20.45%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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/8137/diff?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/8137/diff?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: |
| ... and [1336 more](https://codecov.io/gh/apache/pinot/pull/8137/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/8137?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/8137?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 [a47af49...f12498d](https://codecov.io/gh/apache/pinot/pull/8137?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] mqliang commented on a change in pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r800101265
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/FunctionDefinitionRegistry.java
##########
@@ -29,20 +29,10 @@ private FunctionDefinitionRegistry() {
}
public static boolean isAggFunc(String functionName) {
Review comment:
This function looks like just an alias of `AggregationFunctionType.isAggregationFunction`. Can we just remove it and let caller to call `AggregationFunctionType.isAggregationFunction` directly?
--
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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1030699838
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8137?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 [#8137](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d2ad462) into [master](https://codecov.io/gh/apache/pinot/commit/8bbf93aa4377dbdf597e7940670893330452b33f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bbf93a) will **decrease** coverage by `3.43%`.
> The diff coverage is `96.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137 +/- ##
============================================
- Coverage 71.39% 67.96% -3.44%
+ Complexity 4303 4222 -81
============================================
Files 1624 1224 -400
Lines 84198 61250 -22948
Branches 12602 9469 -3133
============================================
- Hits 60116 41629 -18487
+ Misses 19970 16694 -3276
+ Partials 4112 2927 -1185
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.96% <96.00%> (-0.01%)` | :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/8137?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `91.35% <92.30%> (-5.87%)` | :arrow_down: |
| [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `86.95% <100.00%> (-1.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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/8137/diff?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/8137/diff?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/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ache/pinot/server/access/AccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL2FjY2Vzcy9BY2Nlc3NDb250cm9sRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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: |
| ... and [616 more](https://codecov.io/gh/apache/pinot/pull/8137/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/8137?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/8137?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 [8bbf93a...d2ad462](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1030699838
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8137?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 [#8137](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7f7974e) into [master](https://codecov.io/gh/apache/pinot/commit/a47af499ddb3b878a0bd0c62264c8bcf1cbd1f20?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a47af49) will **increase** coverage by `34.01%`.
> The diff coverage is `90.32%`.
[](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137 +/- ##
=============================================
+ Coverage 30.69% 64.70% +34.01%
- Complexity 0 4303 +4303
=============================================
Files 1613 1578 -35
Lines 83952 82421 -1531
Branches 12597 12433 -164
=============================================
+ Hits 25768 53334 +27566
+ Misses 55889 25277 -30612
- Partials 2295 3810 +1515
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.90% <90.32%> (?)` | |
| unittests2 | `14.17% <0.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/pinot/common/utils/SqlResultComparator.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU3FsUmVzdWx0Q29tcGFyYXRvci5qYXZh) | `0.00% <0.00%> (-18.85%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `90.24% <85.71%> (+90.24%)` | :arrow_up: |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (+10.95%)` | :arrow_up: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `73.36% <100.00%> (+19.09%)` | :arrow_up: |
| [...pql/parsers/PinotQuery2BrokerRequestConverter.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QaW5vdFF1ZXJ5MkJyb2tlclJlcXVlc3RDb252ZXJ0ZXIuamF2YQ==) | `88.51% <100.00%> (+4.72%)` | :arrow_up: |
| [...inot/pql/parsers/pql2/ast/OutputColumnAstNode.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9PdXRwdXRDb2x1bW5Bc3ROb2RlLmphdmE=) | `95.12% <100.00%> (+36.58%)` | :arrow_up: |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `86.95% <100.00%> (+20.45%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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/8137/diff?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/8137/diff?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: |
| ... and [1335 more](https://codecov.io/gh/apache/pinot/pull/8137/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/8137?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/8137?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 [a47af49...7f7974e](https://codecov.io/gh/apache/pinot/pull/8137?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] mqliang commented on a change in pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r800101335
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/FunctionDefinitionRegistry.java
##########
@@ -29,20 +29,10 @@ private FunctionDefinitionRegistry() {
}
public static boolean isAggFunc(String functionName) {
- try {
- AggregationFunctionType.getAggregationFunctionType(functionName);
- return true;
- } catch (IllegalArgumentException e) {
- return false;
- }
+ return AggregationFunctionType.isAggregationFunction(functionName);
}
public static boolean isTransformFunc(String functionName) {
- try {
- TransformFunctionType.getTransformFunctionType(functionName);
- return true;
- } catch (IllegalArgumentException e) {
- return false;
- }
+ return TransformFunctionType.isTransformFunction(functionName);
Review comment:
ditto
--
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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r801898017
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
##########
@@ -81,14 +88,28 @@ public String getName() {
return _name;
}
+ public static boolean isAggregationFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
+ try {
+ getAggregationFunctionType(functionName);
+ return true;
+ } catch (Exception ignore) {
Review comment:
We can directly return `false` here?
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
##########
@@ -81,14 +87,28 @@ public String getName() {
return _name;
}
+ public static boolean isAggregationFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
+ try {
+ getAggregationFunctionType(functionName);
+ return true;
+ } catch (Exception ignore) {
+ }
+ }
+ String upperCaseFunctionName = functionName.replace("_", "").toUpperCase();
+ return NAMES.contains(upperCaseFunctionName);
+ }
+
/**
* Returns the corresponding aggregation function type for the given function name.
* <p>NOTE: Underscores in the function name are ignored.
*/
public static AggregationFunctionType getAggregationFunctionType(String functionName) {
- String upperCaseFunctionName = StringUtils.remove(functionName, '_').toUpperCase();
- if (upperCaseFunctionName.startsWith("PERCENTILE")) {
- String remainingFunctionName = upperCaseFunctionName.substring(10);
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
Review comment:
I believe the allocation is from the `StringUtils.remove(functionName, '_').toUpperCase()` which cannot be avoided in most of cases (non-percentile function).
A little behavior change here if we use `regionMatches` is that `percent_ile` cannot be recognized (not really important though).
Either way is fine, you can make the decision
--
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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1030699838
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8137?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 [#8137](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f12498d) into [master](https://codecov.io/gh/apache/pinot/commit/a47af499ddb3b878a0bd0c62264c8bcf1cbd1f20?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a47af49) will **increase** coverage by `39.61%`.
> The diff coverage is `93.33%`.
[](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137 +/- ##
=============================================
+ Coverage 30.69% 70.31% +39.61%
- Complexity 0 4303 +4303
=============================================
Files 1613 1623 +10
Lines 83952 84313 +361
Branches 12597 12637 +40
=============================================
+ Hits 25768 59281 +33513
+ Misses 55889 20928 -34961
- Partials 2295 4104 +1809
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.81% <26.66%> (-0.02%)` | :arrow_down: |
| integration2 | `?` | |
| unittests1 | `67.89% <93.33%> (?)` | |
| unittests2 | `14.18% <0.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/pinot/common/utils/SqlResultComparator.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU3FsUmVzdWx0Q29tcGFyYXRvci5qYXZh) | `18.84% <0.00%> (ø)` | |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `91.35% <92.30%> (+91.35%)` | :arrow_up: |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (+10.95%)` | :arrow_up: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `74.37% <100.00%> (+20.10%)` | :arrow_up: |
| [...pql/parsers/PinotQuery2BrokerRequestConverter.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QaW5vdFF1ZXJ5MkJyb2tlclJlcXVlc3RDb252ZXJ0ZXIuamF2YQ==) | `89.86% <100.00%> (+6.08%)` | :arrow_up: |
| [...inot/pql/parsers/pql2/ast/OutputColumnAstNode.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9PdXRwdXRDb2x1bW5Bc3ROb2RlLmphdmE=) | `95.12% <100.00%> (+36.58%)` | :arrow_up: |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `87.92% <100.00%> (+21.41%)` | :arrow_up: |
| [...t/core/plan/StreamingInstanceResponsePlanNode.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL1N0cmVhbWluZ0luc3RhbmNlUmVzcG9uc2VQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ore/operator/streaming/StreamingResponseUtils.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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/8137/diff?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: |
| ... and [1161 more](https://codecov.io/gh/apache/pinot/pull/8137/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/8137?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/8137?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 [a47af49...f12498d](https://codecov.io/gh/apache/pinot/pull/8137?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] richardstartin commented on a change in pull request #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r801935690
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
##########
@@ -81,14 +88,28 @@ public String getName() {
return _name;
}
+ public static boolean isAggregationFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
+ try {
+ getAggregationFunctionType(functionName);
+ return true;
+ } catch (Exception ignore) {
Review comment:
agreed
--
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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1030699838
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8137?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 [#8137](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d2ad462) into [master](https://codecov.io/gh/apache/pinot/commit/8bbf93aa4377dbdf597e7940670893330452b33f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bbf93a) will **decrease** coverage by `9.69%`.
> The diff coverage is `96.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137 +/- ##
============================================
- Coverage 71.39% 61.70% -9.70%
+ Complexity 4303 4222 -81
============================================
Files 1624 1613 -11
Lines 84198 83857 -341
Branches 12602 12566 -36
============================================
- Hits 60116 51742 -8374
- Misses 19970 28350 +8380
+ Partials 4112 3765 -347
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.52% <24.00%> (-0.16%)` | :arrow_down: |
| unittests1 | `67.96% <96.00%> (-0.01%)` | :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/8137?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `91.35% <92.30%> (-5.87%)` | :arrow_down: |
| [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `87.92% <100.00%> (-0.03%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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/8137/diff?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/8137/diff?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/8137/diff?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: |
| [...oller/api/resources/PinotControllerAppConfigs.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90Q29udHJvbGxlckFwcENvbmZpZ3MuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ler/recommender/data/generator/BytesGenerator.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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: |
| ... and [286 more](https://codecov.io/gh/apache/pinot/pull/8137/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/8137?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/8137?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 [8bbf93a...d2ad462](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r801003386
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -101,12 +107,28 @@
// Geo indexing
GEOTOH3("geoToH3");
+ private static final Set<String> NAMES = Arrays.stream(values())
+ .flatMap(func -> Stream.of(func.getName(), func.getName().replace("_", "").toUpperCase(),
+ func.getName().toUpperCase(), func.getName().toLowerCase(), func.name(), func.name().toLowerCase()))
+ .collect(Collectors.toSet());
+
private final String _name;
TransformFunctionType(String name) {
_name = name;
}
+ public static boolean isTransformFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ // scalar functions
+ if (FunctionRegistry.containsFunction(functionName)) {
+ return true;
+ }
+ return NAMES.contains(functionName.toUpperCase().replace("_", ""));
Review comment:
Is `replace("_", "")` faster than `StringUtils.remove(functionName, '_')`? Per the implementation, seems `replace("_", "")` is much more costly, and I don't see annotation about it being optimized by JDK
##########
File path: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
##########
@@ -81,14 +87,28 @@ public String getName() {
return _name;
}
+ public static boolean isAggregationFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
+ try {
+ getAggregationFunctionType(functionName);
+ return true;
+ } catch (Exception ignore) {
+ }
+ }
+ String upperCaseFunctionName = functionName.replace("_", "").toUpperCase();
+ return NAMES.contains(upperCaseFunctionName);
+ }
+
/**
* Returns the corresponding aggregation function type for the given function name.
* <p>NOTE: Underscores in the function name are ignored.
*/
public static AggregationFunctionType getAggregationFunctionType(String functionName) {
- String upperCaseFunctionName = StringUtils.remove(functionName, '_').toUpperCase();
- if (upperCaseFunctionName.startsWith("PERCENTILE")) {
- String remainingFunctionName = upperCaseFunctionName.substring(10);
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
Review comment:
Does this give better performance than `startsWith()`? In most cases, the query won't match this if check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1030699838
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8137?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 [#8137](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f12498d) into [master](https://codecov.io/gh/apache/pinot/commit/a47af499ddb3b878a0bd0c62264c8bcf1cbd1f20?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a47af49) will **decrease** coverage by `16.51%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137 +/- ##
=============================================
- Coverage 30.69% 14.18% -16.52%
- Complexity 0 81 +81
=============================================
Files 1613 1578 -35
Lines 83952 82432 -1520
Branches 12597 12433 -164
=============================================
- Hits 25768 11690 -14078
- Misses 55889 69873 +13984
+ Partials 2295 869 -1426
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests2 | `14.18% <0.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `0.00% <0.00%> (-89.05%)` | :arrow_down: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `0.00% <0.00%> (-54.28%)` | :arrow_down: |
| [...apache/pinot/common/utils/SqlResultComparator.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU3FsUmVzdWx0Q29tcGFyYXRvci5qYXZh) | `0.00% <0.00%> (-18.85%)` | :arrow_down: |
| [...pql/parsers/PinotQuery2BrokerRequestConverter.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QaW5vdFF1ZXJ5MkJyb2tlclJlcXVlc3RDb252ZXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-83.79%)` | :arrow_down: |
| [...inot/pql/parsers/pql2/ast/OutputColumnAstNode.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9PdXRwdXRDb2x1bW5Bc3ROb2RlLmphdmE=) | `0.00% <0.00%> (-58.54%)` | :arrow_down: |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `0.00% <0.00%> (-66.51%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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/8137/diff?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: |
| [...a/org/apache/pinot/core/plan/GlobalPlanImplV0.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0dsb2JhbFBsYW5JbXBsVjAuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [820 more](https://codecov.io/gh/apache/pinot/pull/8137/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/8137?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/8137?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 [a47af49...f12498d](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1030699838
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8137?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 [#8137](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d2ad462) into [master](https://codecov.io/gh/apache/pinot/commit/8bbf93aa4377dbdf597e7940670893330452b33f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bbf93a) will **decrease** coverage by `7.22%`.
> The diff coverage is `96.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137 +/- ##
============================================
- Coverage 71.39% 64.17% -7.23%
+ Complexity 4303 4222 -81
============================================
Files 1624 1613 -11
Lines 84198 83857 -341
Branches 12602 12566 -36
============================================
- Hits 60116 53815 -6301
- Misses 19970 26147 +6177
+ Partials 4112 3895 -217
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.82% <24.00%> (-0.05%)` | :arrow_down: |
| integration2 | `27.52% <24.00%> (-0.16%)` | :arrow_down: |
| unittests1 | `67.96% <96.00%> (-0.01%)` | :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/8137?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `91.35% <92.30%> (-5.87%)` | :arrow_down: |
| [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `87.92% <100.00%> (-0.03%)` | :arrow_down: |
| [...pinot/controller/recommender/io/ConfigManager.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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/8137/diff?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/8137/diff?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: |
| [...oller/api/resources/PinotControllerAppConfigs.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90Q29udHJvbGxlckFwcENvbmZpZ3MuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ler/recommender/data/generator/BytesGenerator.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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/8137/diff?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: |
| ... and [210 more](https://codecov.io/gh/apache/pinot/pull/8137/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/8137?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/8137?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 [8bbf93a...d2ad462](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137: do not identify function types by throwing exceptions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#issuecomment-1030699838
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8137?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 [#8137](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7f7974e) into [master](https://codecov.io/gh/apache/pinot/commit/a47af499ddb3b878a0bd0c62264c8bcf1cbd1f20?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a47af49) will **increase** coverage by `39.46%`.
> The diff coverage is `90.32%`.
[](https://codecov.io/gh/apache/pinot/pull/8137?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 #8137 +/- ##
=============================================
+ Coverage 30.69% 70.15% +39.45%
- Complexity 0 4303 +4303
=============================================
Files 1613 1623 +10
Lines 83952 84302 +350
Branches 12597 12637 +40
=============================================
+ Hits 25768 59141 +33373
+ Misses 55889 21071 -34818
- Partials 2295 4090 +1795
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.61% <25.80%> (-0.11%)` | :arrow_down: |
| unittests1 | `67.90% <90.32%> (?)` | |
| unittests2 | `14.17% <0.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8137?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/pinot/common/utils/SqlResultComparator.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU3FsUmVzdWx0Q29tcGFyYXRvci5qYXZh) | `0.00% <0.00%> (-18.85%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `90.24% <85.71%> (+90.24%)` | :arrow_up: |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (+10.95%)` | :arrow_up: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `74.37% <100.00%> (+20.10%)` | :arrow_up: |
| [...pql/parsers/PinotQuery2BrokerRequestConverter.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9QaW5vdFF1ZXJ5MkJyb2tlclJlcXVlc3RDb252ZXJ0ZXIuamF2YQ==) | `89.86% <100.00%> (+6.08%)` | :arrow_up: |
| [...inot/pql/parsers/pql2/ast/OutputColumnAstNode.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9PdXRwdXRDb2x1bW5Bc3ROb2RlLmphdmE=) | `95.12% <100.00%> (+36.58%)` | :arrow_up: |
| [...org/apache/pinot/sql/parsers/CalciteSqlParser.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9DYWxjaXRlU3FsUGFyc2VyLmphdmE=) | `87.92% <100.00%> (+21.41%)` | :arrow_up: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?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: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/8137/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-94.74%)` | :arrow_down: |
| ... and [1168 more](https://codecov.io/gh/apache/pinot/pull/8137/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/8137?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/8137?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 [a47af49...7f7974e](https://codecov.io/gh/apache/pinot/pull/8137?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