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/01 11:34:43 UTC
[GitHub] [pinot] richardstartin opened a new pull request #8100: add least and greatest functions
richardstartin opened a new pull request #8100:
URL: https://github.com/apache/pinot/pull/8100
Closes #8085
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c291d5c) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **increase** coverage by `6.64%`.
> The diff coverage is `83.45%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
============================================
+ Coverage 64.71% 71.36% +6.64%
Complexity 4306 4306
============================================
Files 1572 1620 +48
Lines 82006 84029 +2023
Branches 12330 12574 +244
============================================
+ Hits 53071 59966 +6895
+ Misses 25166 19980 -5186
- Partials 3769 4083 +314
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.95% <3.00%> (?)` | |
| integration2 | `27.54% <3.00%> (?)` | |
| unittests1 | `68.00% <83.45%> (+0.07%)` | :arrow_up: |
| unittests2 | `14.16% <0.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `53.33% <0.00%> (+14.87%)` | :arrow_up: |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `80.95% <80.95%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `80.95% <80.95%> (ø)` | |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `90.69% <90.69%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `82.72% <100.00%> (+3.09%)` | :arrow_up: |
| [...ata/manager/offline/DimensionTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9EaW1lbnNpb25UYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `86.27% <0.00%> (-2.62%)` | :arrow_down: |
| [...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh) | `77.65% <0.00%> (-2.13%)` | :arrow_down: |
| [...e/pinot/segment/local/io/util/PinotDataBitSet.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby91dGlsL1Bpbm90RGF0YUJpdFNldC5qYXZh) | `95.62% <0.00%> (-1.46%)` | :arrow_down: |
| [...ache/pinot/plugin/metrics/yammer/YammerMetric.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1tZXRyaWNzL3Bpbm90LXlhbW1lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL21ldHJpY3MveWFtbWVyL1lhbW1lck1ldHJpYy5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [375 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...c291d5c](https://codecov.io/gh/apache/pinot/pull/8100?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 merged pull request #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin merged pull request #8100:
URL: https://github.com/apache/pinot/pull/8100
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796885572
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
+ dataType = argumentType;
+ }
Review comment:
outdated code, implemented more laborious promotion algorithm
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r797484179
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -54,15 +54,27 @@ public static double mod(double a, double b) {
}
@ScalarFunction
- public static double min(double a, double b) {
+ public static double least(double a, double b) {
return Double.min(a, b);
}
@ScalarFunction
- public static double max(double a, double b) {
+ public static double greatest(double a, double b) {
return Double.max(a, b);
}
+ @Deprecated
Review comment:
min/max were never actually possible to use in SQL because they clashed with the aggregation function. Ingestion transforms aren't specified in SQL, so I'm not sure they really need to be SQL compliant.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796885302
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
Review comment:
I've changed this anyway - try to preserve type, when there are mismatches, most of the time promote to DOUBLE, but promote pairs of LONG and TIMESTAMP to TIMESTAMP, pairs of INT and LONG to LONG.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796854964
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
Review comment:
This will turn `LONG` into `FLOAT`, which seems wrong to me
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
+ dataType = argumentType;
+ }
Review comment:
We might want an else clause here to reject unacceptable combinations
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
+ dataType = argumentType;
+ }
+ }
+ }
+ _resultMetadata = new TransformResultMetadata(dataType, true, false);
+ _arguments = arguments;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return _resultMetadata;
+ }
+
+ @Override
+ public String getName() {
+ return _name;
+ }
+
+ private static EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> createAcceptableCombinations() {
+ EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> combinations = new EnumMap<>(FieldSpec.DataType.class);
+ combinations.put(FieldSpec.DataType.TIMESTAMP, EnumSet.of(FieldSpec.DataType.TIMESTAMP, FieldSpec.DataType.LONG));
Review comment:
Should we allow `TIMESTAMP` to be compared with floating numbers (use `double` to compare)?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -54,15 +54,27 @@ public static double mod(double a, double b) {
}
@ScalarFunction
- public static double min(double a, double b) {
+ public static double least(double a, double b) {
return Double.min(a, b);
}
@ScalarFunction
- public static double max(double a, double b) {
+ public static double greatest(double a, double b) {
return Double.max(a, b);
}
+ @Deprecated
Review comment:
We should deprecate them as they are not standard sql. Let's add some release note in the PR description
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -96,7 +96,10 @@
ST_WITHIN("ST_Within"),
// Geo indexing
- GEOTOH3("geoToH3");
+ GEOTOH3("geoToH3"),
+
+ LEAST("least"),
Review comment:
Suggest moving it above to be along with other arithmetic functions
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3816b7) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **increase** coverage by `3.22%`.
> The diff coverage is `85.82%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
============================================
+ Coverage 64.71% 67.94% +3.22%
+ Complexity 4306 4222 -84
============================================
Files 1572 1224 -348
Lines 82006 61162 -20844
Branches 12330 9452 -2878
============================================
- Hits 53071 41555 -11516
+ Misses 25166 16689 -8477
+ Partials 3769 2918 -851
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `67.94% <85.82%> (+<0.01%)` | :arrow_up: |
| 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/8100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `46.66% <0.00%> (+8.20%)` | :arrow_up: |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `79.06% <79.06%> (ø)` | |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `90.47% <90.47%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `90.47% <90.47%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `80.00% <100.00%> (+0.37%)` | :arrow_up: |
| [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `67.13% <0.00%> (-15.39%)` | :arrow_down: |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| [...pinot/controller/tuner/RealTimeAutoIndexTuner.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci90dW5lci9SZWFsVGltZUF1dG9JbmRleFR1bmVyLmphdmE=) | | |
| ... and [355 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...c3816b7](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3a53d90) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **decrease** coverage by `50.61%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 3a53d90 differs from pull request most recent head 72fc7d6. Consider uploading reports for the commit 72fc7d6 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
=============================================
- Coverage 64.71% 14.10% -50.62%
+ Complexity 4306 81 -4225
=============================================
Files 1572 1575 +3
Lines 82006 82132 +126
Branches 12330 12360 +30
=============================================
- Hits 53071 11584 -41487
- Misses 25166 69683 +44517
+ Partials 3769 865 -2904
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `?` | |
| unittests2 | `14.10% <0.00%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?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/8100/diff?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%> (-100.00%)` | :arrow_down: |
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `0.00% <0.00%> (-38.47%)` | :arrow_down: |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-79.63%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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/8100/diff?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: |
| ... and [1084 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...72fc7d6](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r798915206
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/GreatestTransformFunction.java
##########
@@ -0,0 +1,117 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+
+
+public class GreatestTransformFunction extends SelectTupleElementTransformFunction {
+
+ public GreatestTransformFunction() {
+ super(TransformFunctionType.GREATEST.getName());
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int numDocs = projectionBlock.getNumDocs();
+ if (_intValuesSV == null || _intValuesSV.length < numDocs) {
+ _intValuesSV = new int[numDocs];
+ }
+ int[] values = _arguments.get(0).transformToIntValuesSV(projectionBlock);
+ System.arraycopy(values, 0, _intValuesSV, 0, numDocs);
+ for (int i = 1; i < _arguments.size(); i++) {
+ values = _arguments.get(i).transformToIntValuesSV(projectionBlock);
+ for (int j = 0; j < numDocs & j < values.length; j++) {
Review comment:
No need (and should not) to check the boundary of values, same for other places
```suggestion
for (int j = 0; j < numDocs; j++) {
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,109 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP,
+ FieldSpec.DataType.STRING);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. Required one of " + SUPPORTED_DATATYPES);
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ dataType = getLowestCommonDenominatorType(dataType, argumentType);
+ } else {
+ throw new IllegalArgumentException(
+ "combination " + argumentType + " not supported. Required one of " + ACCEPTABLE_COMBINATIONS.get(dataType));
+ }
+ }
+ _resultMetadata = new TransformResultMetadata(dataType, true, false);
+ _arguments = arguments;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return _resultMetadata;
+ }
+
+ @Override
+ public String getName() {
+ return _name;
+ }
+
+ private static FieldSpec.DataType getLowestCommonDenominatorType(FieldSpec.DataType left, FieldSpec.DataType right) {
+ if (left == null || left == right) {
Review comment:
(minor) `left` can never be `null`
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r798930887
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,109 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP,
+ FieldSpec.DataType.STRING);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. Required one of " + SUPPORTED_DATATYPES);
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ dataType = getLowestCommonDenominatorType(dataType, argumentType);
+ } else {
+ throw new IllegalArgumentException(
+ "combination " + argumentType + " not supported. Required one of " + ACCEPTABLE_COMBINATIONS.get(dataType));
+ }
+ }
+ _resultMetadata = new TransformResultMetadata(dataType, true, false);
+ _arguments = arguments;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return _resultMetadata;
+ }
+
+ @Override
+ public String getName() {
+ return _name;
+ }
+
+ private static FieldSpec.DataType getLowestCommonDenominatorType(FieldSpec.DataType left, FieldSpec.DataType right) {
+ if (left == null || left == right) {
Review comment:
how it's currently called, sure.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (45755f6) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **increase** coverage by `5.56%`.
> The diff coverage is `82.35%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
============================================
+ Coverage 64.71% 70.28% +5.56%
+ Complexity 4306 4302 -4
============================================
Files 1572 1624 +52
Lines 82006 84108 +2102
Branches 12330 12591 +261
============================================
+ Hits 53071 59113 +6042
+ Misses 25166 20914 -4252
- Partials 3769 4081 +312
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.81% <2.61%> (?)` | |
| unittests1 | `67.95% <82.35%> (+0.02%)` | :arrow_up: |
| unittests2 | `14.15% <0.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `53.33% <0.00%> (+14.87%)` | :arrow_up: |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `81.13% <81.13%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `81.13% <81.13%> (ø)` | |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `87.80% <87.80%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `82.72% <100.00%> (+3.09%)` | :arrow_up: |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| [...pinot/broker/broker/BrokerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jyb2tlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `88.37% <0.00%> (-4.32%)` | :arrow_down: |
| [...r/recommender/data/generator/GeneratorFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9kYXRhL2dlbmVyYXRvci9HZW5lcmF0b3JGYWN0b3J5LmphdmE=) | `31.81% <0.00%> (-3.19%)` | :arrow_down: |
| ... and [351 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...45755f6](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796931635
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
Review comment:
That’s what it does.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796867249
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
+ dataType = argumentType;
+ }
+ }
+ }
+ _resultMetadata = new TransformResultMetadata(dataType, true, false);
+ _arguments = arguments;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return _resultMetadata;
+ }
+
+ @Override
+ public String getName() {
+ return _name;
+ }
+
+ private static EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> createAcceptableCombinations() {
+ EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> combinations = new EnumMap<>(FieldSpec.DataType.class);
+ combinations.put(FieldSpec.DataType.TIMESTAMP, EnumSet.of(FieldSpec.DataType.TIMESTAMP, FieldSpec.DataType.LONG));
Review comment:
I don't think there is a good reason for a user to want to do that.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796883706
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
+ dataType = argumentType;
+ }
Review comment:
I've changed this anyway - try to preserve type, when there are mismatches, most of the time promote to DOUBLE, but promote pairs of LONG and TIMESTAMP to TIMESTAMP, pairs of INT and LONG to LONG.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r798929924
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/GreatestTransformFunction.java
##########
@@ -0,0 +1,117 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+
+
+public class GreatestTransformFunction extends SelectTupleElementTransformFunction {
+
+ public GreatestTransformFunction() {
+ super(TransformFunctionType.GREATEST.getName());
+ }
+
+ @Override
+ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+ int numDocs = projectionBlock.getNumDocs();
+ if (_intValuesSV == null || _intValuesSV.length < numDocs) {
+ _intValuesSV = new int[numDocs];
+ }
+ int[] values = _arguments.get(0).transformToIntValuesSV(projectionBlock);
+ System.arraycopy(values, 0, _intValuesSV, 0, numDocs);
+ for (int i = 1; i < _arguments.size(); i++) {
+ values = _arguments.get(i).transformToIntValuesSV(projectionBlock);
+ for (int j = 0; j < numDocs & j < values.length; j++) {
Review comment:
Actually I did this for a reason - the invariant `numDocs < values.length` is not maintained anywhere and enforcing it allows bounds checks to be removed.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796866822
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
+ dataType = argumentType;
+ }
Review comment:
There won't be unacceptable combinations if we get inside this condition. What this logic does is promote the type used for comparison. If the type of the most recent argument dominates the type of the current argument, it is not updated. If the current type dominates the most recent type, it is updated.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796863568
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
Review comment:
I realise this promotes `LONG` to `FLOAT` as this was my intention, but why is it wrong? What would you do instead and what are the relative merits of your alternative?
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3816b7) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **increase** coverage by `0.00%`.
> The diff coverage is `85.82%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
==========================================
Coverage 64.71% 64.72%
+ Complexity 4306 4303 -3
==========================================
Files 1572 1575 +3
Lines 82006 82139 +133
Branches 12330 12361 +31
==========================================
+ Hits 53071 53164 +93
- Misses 25166 25194 +28
- Partials 3769 3781 +12
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `67.94% <85.82%> (+<0.01%)` | :arrow_up: |
| unittests2 | `14.13% <0.00%> (-0.03%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `46.66% <0.00%> (+8.20%)` | :arrow_up: |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `79.06% <79.06%> (ø)` | |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `90.47% <90.47%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `90.47% <90.47%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `80.00% <100.00%> (+0.37%)` | :arrow_up: |
| [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `67.13% <0.00%> (-15.39%)` | :arrow_down: |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `37.14% <0.00%> (-12.86%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [10 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...c3816b7](https://codecov.io/gh/apache/pinot/pull/8100?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] walterddr commented on a change in pull request #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
walterddr commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796735038
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/TupleSelectionTransformFunctionsTest.java
##########
@@ -0,0 +1,284 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.request.context.RequestContextUtils;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.exception.BadQueryRequestException;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+
+public class TupleSelectionTransformFunctionsTest extends BaseTransformFunctionTest {
Review comment:
nit: can we can simplify this test with a data provider?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -54,15 +54,27 @@ public static double mod(double a, double b) {
}
@ScalarFunction
- public static double min(double a, double b) {
+ public static double least(double a, double b) {
return Double.min(a, b);
}
@ScalarFunction
- public static double max(double a, double b) {
+ public static double greatest(double a, double b) {
return Double.max(a, b);
}
+ @Deprecated
Review comment:
this deprecation warning will not appear to users of sql, so probably we need a release note or a doc update
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796789390
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -54,15 +54,27 @@ public static double mod(double a, double b) {
}
@ScalarFunction
- public static double min(double a, double b) {
+ public static double least(double a, double b) {
return Double.min(a, b);
}
@ScalarFunction
- public static double max(double a, double b) {
+ public static double greatest(double a, double b) {
return Double.max(a, b);
}
+ @Deprecated
Review comment:
We could just remove the deprecation and tolerate some duplication. What do you think?
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796929462
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
Review comment:
mixing `INT`, `LONG`, `TIMESTAMP` -> `LONG`
mixing with floating numbers -> `DOUBLE` (try to keep all the precision, might lose precision for `LONG` though)
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796885007
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/SelectTupleElementTransformFunction.java
##########
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.Comparator;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public abstract class SelectTupleElementTransformFunction extends BaseTransformFunction {
+
+ private static final EnumSet<FieldSpec.DataType> SUPPORTED_DATATYPES = EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT, FieldSpec.DataType.DOUBLE, FieldSpec.DataType.TIMESTAMP);
+
+ private static final Comparator<FieldSpec.DataType> DOMINANCE_HIERARCHY = Comparator.comparingInt(Enum::ordinal);
+
+ private static final EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> ACCEPTABLE_COMBINATIONS =
+ createAcceptableCombinations();
+
+ private final String _name;
+
+ protected List<TransformFunction> _arguments;
+ private TransformResultMetadata _resultMetadata;
+
+ public SelectTupleElementTransformFunction(String name) {
+ _name = name;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ if (arguments.isEmpty()) {
+ throw new IllegalArgumentException(_name + " takes at least one argument");
+ }
+ FieldSpec.DataType dataType = null;
+ for (int i = 0; i < arguments.size(); i++) {
+ TransformFunction argument = arguments.get(i);
+ TransformResultMetadata metadata = argument.getResultMetadata();
+ if (!metadata.isSingleValue()) {
+ throw new IllegalArgumentException(argument.getName() + " at position " + i + " is not single value");
+ }
+ FieldSpec.DataType argumentType = metadata.getDataType();
+ if (!SUPPORTED_DATATYPES.contains(argumentType)) {
+ throw new IllegalArgumentException(argumentType + " not supported. " + SUPPORTED_DATATYPES + " are supported.");
+ }
+ if (dataType == null) {
+ dataType = argumentType;
+ } else if (ACCEPTABLE_COMBINATIONS.get(dataType).contains(argumentType)) {
+ if (DOMINANCE_HIERARCHY.compare(dataType, argumentType) < 0) {
+ dataType = argumentType;
+ }
+ }
+ }
+ _resultMetadata = new TransformResultMetadata(dataType, true, false);
+ _arguments = arguments;
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return _resultMetadata;
+ }
+
+ @Override
+ public String getName() {
+ return _name;
+ }
+
+ private static EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> createAcceptableCombinations() {
+ EnumMap<FieldSpec.DataType, EnumSet<FieldSpec.DataType>> combinations = new EnumMap<>(FieldSpec.DataType.class);
+ combinations.put(FieldSpec.DataType.TIMESTAMP, EnumSet.of(FieldSpec.DataType.TIMESTAMP, FieldSpec.DataType.LONG));
Review comment:
this will now happen
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2a5c4c3) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **decrease** coverage by `50.59%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
=============================================
- Coverage 64.71% 14.11% -50.60%
+ Complexity 4306 81 -4225
=============================================
Files 1572 1575 +3
Lines 82006 82126 +120
Branches 12330 12364 +34
=============================================
- Hits 53071 11593 -41478
- Misses 25166 69665 +44499
+ Partials 3769 868 -2901
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `?` | |
| unittests2 | `14.11% <0.00%> (-0.05%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?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/8100/diff?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%> (-100.00%)` | :arrow_down: |
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `0.00% <0.00%> (-38.47%)` | :arrow_down: |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-79.63%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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/8100/diff?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: |
| ... and [1086 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...2a5c4c3](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r797484179
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -54,15 +54,27 @@ public static double mod(double a, double b) {
}
@ScalarFunction
- public static double min(double a, double b) {
+ public static double least(double a, double b) {
return Double.min(a, b);
}
@ScalarFunction
- public static double max(double a, double b) {
+ public static double greatest(double a, double b) {
return Double.max(a, b);
}
+ @Deprecated
Review comment:
min/max were never actually possible to use in SQL because they clashed with the aggression function. Ingestion transforms aren't specified in SQL, so I'm not sure they really need to be SQL compliant.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (45755f6) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **increase** coverage by `6.59%`.
> The diff coverage is `82.35%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
============================================
+ Coverage 64.71% 71.31% +6.59%
+ Complexity 4306 4302 -4
============================================
Files 1572 1624 +52
Lines 82006 84108 +2102
Branches 12330 12591 +261
============================================
+ Hits 53071 59978 +6907
+ Misses 25166 20024 -5142
- Partials 3769 4106 +337
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.81% <2.61%> (?)` | |
| integration2 | `27.60% <2.61%> (?)` | |
| unittests1 | `67.95% <82.35%> (+0.02%)` | :arrow_up: |
| unittests2 | `14.15% <0.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `53.33% <0.00%> (+14.87%)` | :arrow_up: |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `81.13% <81.13%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `81.13% <81.13%> (ø)` | |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `87.80% <87.80%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `82.72% <100.00%> (+3.09%)` | :arrow_up: |
| [...nt/local/startree/v2/store/StarTreeDataSource.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURhdGFTb3VyY2UuamF2YQ==) | `40.00% <0.00%> (-13.34%)` | :arrow_down: |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `72.22% <0.00%> (-5.56%)` | :arrow_down: |
| [...r/recommender/data/generator/GeneratorFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9kYXRhL2dlbmVyYXRvci9HZW5lcmF0b3JGYWN0b3J5LmphdmE=) | `31.81% <0.00%> (-3.19%)` | :arrow_down: |
| [...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `78.53% <0.00%> (-2.62%)` | :arrow_down: |
| ... and [387 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...45755f6](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r797483416
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/TupleSelectionTransformFunctionsTest.java
##########
@@ -0,0 +1,284 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.request.context.RequestContextUtils;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.exception.BadQueryRequestException;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+
+public class TupleSelectionTransformFunctionsTest extends BaseTransformFunctionTest {
Review comment:
I've simplified some of them with a data provider for rejected cases
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c291d5c) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **increase** coverage by `5.59%`.
> The diff coverage is `83.45%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
============================================
+ Coverage 64.71% 70.31% +5.59%
Complexity 4306 4306
============================================
Files 1572 1620 +48
Lines 82006 84029 +2023
Branches 12330 12574 +244
============================================
+ Hits 53071 59083 +6012
+ Misses 25166 20883 -4283
- Partials 3769 4063 +294
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.79% <3.00%> (?)` | |
| unittests1 | `68.00% <83.45%> (+0.07%)` | :arrow_up: |
| unittests2 | `14.16% <0.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `53.33% <0.00%> (+14.87%)` | :arrow_up: |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `80.95% <80.95%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `80.95% <80.95%> (ø)` | |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `90.69% <90.69%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `82.72% <100.00%> (+3.09%)` | :arrow_up: |
| [...ata/manager/offline/DimensionTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9EaW1lbnNpb25UYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `86.27% <0.00%> (-2.62%)` | :arrow_down: |
| [...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh) | `77.65% <0.00%> (-2.13%)` | :arrow_down: |
| [...e/pinot/segment/local/io/util/PinotDataBitSet.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby91dGlsL1Bpbm90RGF0YUJpdFNldC5qYXZh) | `95.62% <0.00%> (-1.46%)` | :arrow_down: |
| [...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `80.62% <0.00%> (-0.53%)` | :arrow_down: |
| ... and [339 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...c291d5c](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c291d5c) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **increase** coverage by `6.61%`.
> The diff coverage is `83.45%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
============================================
+ Coverage 64.71% 71.32% +6.61%
Complexity 4306 4306
============================================
Files 1572 1620 +48
Lines 82006 84029 +2023
Branches 12330 12574 +244
============================================
+ Hits 53071 59936 +6865
+ Misses 25166 20006 -5160
- Partials 3769 4087 +318
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `28.79% <3.00%> (?)` | |
| integration2 | `27.54% <3.00%> (?)` | |
| unittests1 | `68.00% <83.45%> (+0.07%)` | :arrow_up: |
| unittests2 | `14.16% <0.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `53.33% <0.00%> (+14.87%)` | :arrow_up: |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `80.95% <80.95%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `80.95% <80.95%> (ø)` | |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `90.69% <90.69%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `82.72% <100.00%> (+3.09%)` | :arrow_up: |
| [...ata/manager/offline/DimensionTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9EaW1lbnNpb25UYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `86.27% <0.00%> (-2.62%)` | :arrow_down: |
| [...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh) | `77.65% <0.00%> (-2.13%)` | :arrow_down: |
| [...e/pinot/segment/local/io/util/PinotDataBitSet.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby91dGlsL1Bpbm90RGF0YUJpdFNldC5qYXZh) | `95.62% <0.00%> (-1.46%)` | :arrow_down: |
| [...t/server/api/resources/DefaultExceptionMapper.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9EZWZhdWx0RXhjZXB0aW9uTWFwcGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [375 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...c291d5c](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#discussion_r796765705
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/TupleSelectionTransformFunctionsTest.java
##########
@@ -0,0 +1,284 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.transform.function;
+
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.request.context.RequestContextUtils;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.exception.BadQueryRequestException;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+
+public class TupleSelectionTransformFunctionsTest extends BaseTransformFunctionTest {
Review comment:
As I hope you've noticed, I like DataProviders, but I don't see a way here, the tests look deceptively similar but the duplication is because of primitive types.
--
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 #8100: add least and greatest functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8100:
URL: https://github.com/apache/pinot/pull/8100#issuecomment-1026775031
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8100?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 [#8100](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2a5c4c3) into [master](https://codecov.io/gh/apache/pinot/commit/71e28a2313a0e175e64398b195e488b0fd67d49b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e28a2) will **increase** coverage by `0.02%`.
> The diff coverage is `81.20%`.
[](https://codecov.io/gh/apache/pinot/pull/8100?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 #8100 +/- ##
============================================
+ Coverage 64.71% 64.74% +0.02%
+ Complexity 4306 4305 -1
============================================
Files 1572 1575 +3
Lines 82006 82126 +120
Branches 12330 12364 +34
============================================
+ Hits 53071 53169 +98
- Misses 25166 25169 +3
- Partials 3769 3788 +19
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `67.99% <81.20%> (+0.05%)` | :arrow_up: |
| unittests2 | `14.11% <0.00%> (-0.05%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ArithmeticFunctions.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0FyaXRobWV0aWNGdW5jdGlvbnMuamF2YQ==) | `46.66% <0.00%> (+8.20%)` | :arrow_up: |
| [.../transform/function/GreatestTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR3JlYXRlc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `80.95% <80.95%> (ø)` | |
| [...tor/transform/function/LeastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTGVhc3RUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `80.95% <80.95%> (ø)` | |
| [.../function/SelectTupleElementTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2VsZWN0VHVwbGVFbGVtZW50VHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `83.72% <83.72%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?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%> (ø)` | |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `80.00% <100.00%> (+0.37%)` | :arrow_up: |
| [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `37.14% <0.00%> (-12.86%)` | :arrow_down: |
| [...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | `73.29% <0.00%> (-7.86%)` | :arrow_down: |
| [.../pinot/core/query/scheduler/PriorityScheduler.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvUHJpb3JpdHlTY2hlZHVsZXIuamF2YQ==) | `80.82% <0.00%> (-2.74%)` | :arrow_down: |
| [...ata/manager/offline/DimensionTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/8100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9EaW1lbnNpb25UYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `86.27% <0.00%> (-2.62%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/apache/pinot/pull/8100/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/8100?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/8100?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 [71e28a2...2a5c4c3](https://codecov.io/gh/apache/pinot/pull/8100?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