You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/05/19 04:59:51 UTC
[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #6941: Fix CAST transform function for chained transforms
Jackie-Jiang opened a new pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941
## Description
Currently the `CAST` transform function relies on the caller to call the correct API to perform the cast. When the `CAST` is chained with other transform functions such as `ADD`, the next transform function might ignore the output type, and ignore the `CAST`. E.g. `CAST(doubleCol AS INT) + 5` will return the same result as `doubleCol + 5`, which is unexpected.
This PR fixes the bug by checking the result type and always cast the values.
Also fix a bug for `TIMESTAMP` cast (missing `break;`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter commented on pull request #6941: Fix CAST transform function for chained transforms
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941#issuecomment-843763050
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 [#6941](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (88cf497) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/9becc57eec981d71d5b45af5da7b720840d18f17?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9becc57) will **decrease** coverage by `8.01%`.
> The diff coverage is `41.75%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 #6941 +/- ##
============================================
- Coverage 73.41% 65.40% -8.02%
Complexity 12 12
============================================
Files 1431 1431
Lines 70689 70765 +76
Branches 10232 10252 +20
============================================
- Hits 51897 46282 -5615
- Misses 15344 21157 +5813
+ Partials 3448 3326 -122
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `?` | `?` | |
| unittests | `65.40% <41.75%> (-0.04%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ator/transform/function/BaseTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQmFzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `36.55% <ø> (-2.35%)` | `0.00 <0.00> (ø)` | |
| [...ator/transform/function/CastTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQ2FzdFRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `43.65% <41.75%> (-16.35%)` | `0.00 <0.00> (ø)` | |
| [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| ... and [339 more](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 [9becc57...88cf497](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #6941: Fix CAST transform function for chained transforms
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941#issuecomment-843795027
@yupeng9 Added more tests and find the issue where `cast double -> long -> int` is not the same as `cast double -> int` when the value is out of the int range. Fixed it by always cast first regardless of the type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #6941: Fix CAST transform function for chained transforms
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] yupeng9 commented on a change in pull request #6941: Fix CAST transform function for chained transforms
Posted by GitBox <gi...@apache.org>.
yupeng9 commented on a change in pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941#discussion_r634922225
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java
##########
@@ -94,51 +94,157 @@ public TransformResultMetadata getResultMetadata() {
@Override
public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToIntValuesSV(projectionBlock);
+ // When casting to FLOAT, need to first read as the result type then convert to int values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.FLOAT) {
+ return _transformFunction.transformToIntValuesSV(projectionBlock);
+ } else {
+ if (_intValuesSV == null) {
+ _intValuesSV = new int[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ float[] floatValues = _transformFunction.transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _intValuesSV, numDocs);
+ return _intValuesSV;
+ }
}
@Override
public long[] transformToLongValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToLongValuesSV(projectionBlock);
+ // When casting to INT/FLOAT/DOUBLE, need to first read as the result type then convert to long values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.FLOAT && resultStoredType != DataType.DOUBLE) {
+ return _transformFunction.transformToLongValuesSV(projectionBlock);
+ } else {
+ if (_longValuesSV == null) {
+ _longValuesSV = new long[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
+ int[] intValues = _transformFunction.transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _longValuesSV, numDocs);
+ } else if (resultStoredType == DataType.FLOAT) {
+ float[] floatValues = _transformFunction.transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _longValuesSV, numDocs);
+ } else {
+ double[] doubleValues = _transformFunction.transformToDoubleValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(doubleValues, _longValuesSV, numDocs);
+ }
+ return _longValuesSV;
+ }
}
@Override
public float[] transformToFloatValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToFloatValuesSV(projectionBlock);
+ // When casting to INT/LONG, need to first read as the result type then convert to float values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.LONG) {
+ return _transformFunction.transformToFloatValuesSV(projectionBlock);
+ } else {
+ if (_floatValuesSV == null) {
+ _floatValuesSV = new float[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
Review comment:
do we need handling of long type?
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/CastTransformFunctionTest.java
##########
@@ -40,21 +40,21 @@ public void testCastTransformFunction() {
testTransformFunction(transformFunction, expectedValues);
expression = RequestContextUtils.getExpressionFromSQL(
- String.format("CAST(ADD(CAST(%s AS INT), %s) AS STRING)", STRING_SV_COLUMN, DOUBLE_SV_COLUMN));
+ String.format("CAST(ADD(CAST(%s AS LONG), %s) AS STRING)", DOUBLE_SV_COLUMN, LONG_SV_COLUMN));
Review comment:
can you add more test coverage? There are quite a few special handling logic added.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java
##########
@@ -94,51 +94,157 @@ public TransformResultMetadata getResultMetadata() {
@Override
public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToIntValuesSV(projectionBlock);
+ // When casting to FLOAT, need to first read as the result type then convert to int values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.FLOAT) {
+ return _transformFunction.transformToIntValuesSV(projectionBlock);
+ } else {
+ if (_intValuesSV == null) {
+ _intValuesSV = new int[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ float[] floatValues = _transformFunction.transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _intValuesSV, numDocs);
+ return _intValuesSV;
+ }
}
@Override
public long[] transformToLongValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToLongValuesSV(projectionBlock);
+ // When casting to INT/FLOAT/DOUBLE, need to first read as the result type then convert to long values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.FLOAT && resultStoredType != DataType.DOUBLE) {
+ return _transformFunction.transformToLongValuesSV(projectionBlock);
+ } else {
+ if (_longValuesSV == null) {
+ _longValuesSV = new long[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
+ int[] intValues = _transformFunction.transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _longValuesSV, numDocs);
+ } else if (resultStoredType == DataType.FLOAT) {
+ float[] floatValues = _transformFunction.transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _longValuesSV, numDocs);
+ } else {
+ double[] doubleValues = _transformFunction.transformToDoubleValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(doubleValues, _longValuesSV, numDocs);
+ }
+ return _longValuesSV;
+ }
}
@Override
public float[] transformToFloatValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToFloatValuesSV(projectionBlock);
+ // When casting to INT/LONG, need to first read as the result type then convert to float values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.LONG) {
+ return _transformFunction.transformToFloatValuesSV(projectionBlock);
+ } else {
+ if (_floatValuesSV == null) {
+ _floatValuesSV = new float[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
+ int[] intValues = _transformFunction.transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _floatValuesSV, numDocs);
+ } else {
+ long[] longValues = _transformFunction.transformToLongValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(longValues, _floatValuesSV, numDocs);
+ }
+ return _floatValuesSV;
+ }
}
@Override
public double[] transformToDoubleValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToDoubleValuesSV(projectionBlock);
+ // When casting to INT/LONG/FLOAT, need to first read as the result type then convert to double values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.LONG && resultStoredType != DataType.FLOAT) {
+ return _transformFunction.transformToDoubleValuesSV(projectionBlock);
+ } else {
+ if (_doubleValuesSV == null) {
+ _doubleValuesSV = new double[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
+ int[] intValues = _transformFunction.transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _doubleValuesSV, numDocs);
+ } else if (resultStoredType == DataType.LONG) {
+ long[] longValues = _transformFunction.transformToLongValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(longValues, _doubleValuesSV, numDocs);
+ } else {
Review comment:
do we need to handle float type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6941: Fix CAST transform function for chained transforms
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941#issuecomment-843763050
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 [#6941](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (88cf497) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/9becc57eec981d71d5b45af5da7b720840d18f17?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9becc57) will **decrease** coverage by `0.00%`.
> The diff coverage is `42.85%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 #6941 +/- ##
============================================
- Coverage 73.41% 73.41% -0.01%
Complexity 12 12
============================================
Files 1431 1431
Lines 70689 70765 +76
Branches 10232 10252 +20
============================================
+ Hits 51897 51951 +54
- Misses 15344 15353 +9
- Partials 3448 3461 +13
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `43.00% <9.89%> (-0.06%)` | `7.00 <0.00> (ø)` | |
| unittests | `65.40% <41.75%> (-0.04%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ator/transform/function/BaseTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQmFzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `38.90% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...ator/transform/function/CastTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQ2FzdFRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `44.44% <42.85%> (-15.56%)` | `0.00 <0.00> (ø)` | |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `28.57% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
| [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | `0.00% <0.00%> (ø%)` | |
| [...nction/DistinctCountBitmapAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50Qml0bWFwQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `41.75% <0.00%> (-7.74%)` | `0.00% <0.00%> (ø%)` | |
| [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `63.85% <0.00%> (-6.03%)` | `0.00% <0.00%> (ø%)` | |
| [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `54.21% <0.00%> (-6.03%)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/pinot/core/startree/StarTreeUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9TdGFyVHJlZVV0aWxzLmphdmE=) | `73.58% <0.00%> (-3.78%)` | `0.00% <0.00%> (ø%)` | |
| [...nMaxValueBasedSelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL01pbk1heFZhbHVlQmFzZWRTZWxlY3Rpb25PcmRlckJ5Q29tYmluZU9wZXJhdG9yLmphdmE=) | `68.65% <0.00%> (-2.24%)` | `0.00% <0.00%> (ø%)` | |
| [...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?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) | `74.72% <0.00%> (-2.20%)` | `0.00% <0.00%> (ø%)` | |
| ... and [23 more](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 [9becc57...88cf497](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6941: Fix CAST transform function for chained transforms
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941#issuecomment-843763050
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 [#6941](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6b92e84) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/9becc57eec981d71d5b45af5da7b720840d18f17?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9becc57) will **decrease** coverage by `7.97%`.
> The diff coverage is `67.76%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 #6941 +/- ##
============================================
- Coverage 73.41% 65.44% -7.98%
Complexity 12 12
============================================
Files 1431 1431
Lines 70689 70797 +108
Branches 10232 10251 +19
============================================
- Hits 51897 46330 -5567
- Misses 15344 21142 +5798
+ Partials 3448 3325 -123
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `?` | `?` | |
| unittests | `65.44% <67.76%> (+<0.01%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ator/transform/function/BaseTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQmFzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `37.07% <ø> (-1.83%)` | `0.00 <0.00> (ø)` | |
| [...ator/transform/function/CastTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQ2FzdFRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `63.92% <67.76%> (+3.92%)` | `0.00 <0.00> (ø)` | |
| [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| ... and [339 more](https://codecov.io/gh/apache/incubator-pinot/pull/6941/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6941?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 [9becc57...6b92e84](https://codecov.io/gh/apache/incubator-pinot/pull/6941?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6941: Fix CAST transform function for chained transforms
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941#discussion_r634938732
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java
##########
@@ -94,51 +94,157 @@ public TransformResultMetadata getResultMetadata() {
@Override
public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToIntValuesSV(projectionBlock);
+ // When casting to FLOAT, need to first read as the result type then convert to int values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.FLOAT) {
+ return _transformFunction.transformToIntValuesSV(projectionBlock);
+ } else {
+ if (_intValuesSV == null) {
+ _intValuesSV = new int[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ float[] floatValues = _transformFunction.transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _intValuesSV, numDocs);
+ return _intValuesSV;
+ }
}
@Override
public long[] transformToLongValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToLongValuesSV(projectionBlock);
+ // When casting to INT/FLOAT/DOUBLE, need to first read as the result type then convert to long values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.FLOAT && resultStoredType != DataType.DOUBLE) {
+ return _transformFunction.transformToLongValuesSV(projectionBlock);
+ } else {
+ if (_longValuesSV == null) {
+ _longValuesSV = new long[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
+ int[] intValues = _transformFunction.transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _longValuesSV, numDocs);
+ } else if (resultStoredType == DataType.FLOAT) {
+ float[] floatValues = _transformFunction.transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _longValuesSV, numDocs);
+ } else {
+ double[] doubleValues = _transformFunction.transformToDoubleValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(doubleValues, _longValuesSV, numDocs);
+ }
+ return _longValuesSV;
+ }
}
@Override
public float[] transformToFloatValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToFloatValuesSV(projectionBlock);
+ // When casting to INT/LONG, need to first read as the result type then convert to float values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.LONG) {
+ return _transformFunction.transformToFloatValuesSV(projectionBlock);
+ } else {
+ if (_floatValuesSV == null) {
+ _floatValuesSV = new float[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
Review comment:
That is in the `else` block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6941: Fix CAST transform function for chained transforms
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6941:
URL: https://github.com/apache/incubator-pinot/pull/6941#discussion_r634938925
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java
##########
@@ -94,51 +94,157 @@ public TransformResultMetadata getResultMetadata() {
@Override
public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToIntValuesSV(projectionBlock);
+ // When casting to FLOAT, need to first read as the result type then convert to int values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.FLOAT) {
+ return _transformFunction.transformToIntValuesSV(projectionBlock);
+ } else {
+ if (_intValuesSV == null) {
+ _intValuesSV = new int[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ float[] floatValues = _transformFunction.transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _intValuesSV, numDocs);
+ return _intValuesSV;
+ }
}
@Override
public long[] transformToLongValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToLongValuesSV(projectionBlock);
+ // When casting to INT/FLOAT/DOUBLE, need to first read as the result type then convert to long values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.FLOAT && resultStoredType != DataType.DOUBLE) {
+ return _transformFunction.transformToLongValuesSV(projectionBlock);
+ } else {
+ if (_longValuesSV == null) {
+ _longValuesSV = new long[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
+ int[] intValues = _transformFunction.transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _longValuesSV, numDocs);
+ } else if (resultStoredType == DataType.FLOAT) {
+ float[] floatValues = _transformFunction.transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _longValuesSV, numDocs);
+ } else {
+ double[] doubleValues = _transformFunction.transformToDoubleValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(doubleValues, _longValuesSV, numDocs);
+ }
+ return _longValuesSV;
+ }
}
@Override
public float[] transformToFloatValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToFloatValuesSV(projectionBlock);
+ // When casting to INT/LONG, need to first read as the result type then convert to float values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.LONG) {
+ return _transformFunction.transformToFloatValuesSV(projectionBlock);
+ } else {
+ if (_floatValuesSV == null) {
+ _floatValuesSV = new float[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
+ int[] intValues = _transformFunction.transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _floatValuesSV, numDocs);
+ } else {
+ long[] longValues = _transformFunction.transformToLongValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(longValues, _floatValuesSV, numDocs);
+ }
+ return _floatValuesSV;
+ }
}
@Override
public double[] transformToDoubleValuesSV(ProjectionBlock projectionBlock) {
- return _transformFunction.transformToDoubleValuesSV(projectionBlock);
+ // When casting to INT/LONG/FLOAT, need to first read as the result type then convert to double values
+ DataType resultStoredType = _resultMetadata.getDataType().getStoredType();
+ if (resultStoredType != DataType.INT && resultStoredType != DataType.LONG && resultStoredType != DataType.FLOAT) {
+ return _transformFunction.transformToDoubleValuesSV(projectionBlock);
+ } else {
+ if (_doubleValuesSV == null) {
+ _doubleValuesSV = new double[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+ int numDocs = projectionBlock.getNumDocs();
+ if (resultStoredType == DataType.INT) {
+ int[] intValues = _transformFunction.transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _doubleValuesSV, numDocs);
+ } else if (resultStoredType == DataType.LONG) {
+ long[] longValues = _transformFunction.transformToLongValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(longValues, _doubleValuesSV, numDocs);
+ } else {
Review comment:
Yes, and that is in the last `else` block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org