You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2026/08/05 18:41:08 UTC
[PR] Render UUID casts as canonical strings [pinot]
xiangfu0 opened a new pull request, #19165:
URL: https://github.com/apache/pinot/pull/19165
## What
- Render `CAST(uuid AS STRING)` with the canonical dashed UUID representation.
- Handle both single-value and multi-value UUID expressions.
- Add focused runtime transform coverage.
## Why
UUID is logically distinct from BYTES but uses 16-byte storage. The generic BYTES-to-STRING path renders those bytes as a 32-character hex string. This preserves UUID semantics while leaving ordinary BYTES casts unchanged.
## Validation
- `CastTransformFunctionTest` passed before the base-only rebase.
- `spotless:apply`, `license:format`, `checkstyle:check`, and `license:check` on `pinot-core`.
--
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
Re: [PR] Render UUID casts as canonical strings [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #19165:
URL: https://github.com/apache/pinot/pull/19165#discussion_r3723240839
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/CastTransformFunctionTest.java:
##########
@@ -35,6 +35,17 @@
public class CastTransformFunctionTest extends BaseTransformFunctionTest {
+ @Test
+ public void testCastUuidToString() {
+ String uuid = "550e8400-e29b-41d4-a716-446655440000";
+ ExpressionContext expression = RequestContextUtils.getExpression("CAST(toUuid('" + uuid + "') AS STRING)");
+ TransformFunction transformFunction = TransformFunctionFactory.get(expression, _dataSourceMap);
+
+ String[] expectedValues = new String[NUM_ROWS];
+ Arrays.fill(expectedValues, uuid);
+ testTransformFunction(transformFunction, expectedValues);
+ }
Review Comment:
The PR description says it adds coverage for both single-value and multi-value UUID expressions, but this test only covers the single-value path. Either add a corresponding MV test (e.g., casting an MV UUID expression to STRING) or adjust the PR description to reflect the current test scope.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java:
##########
@@ -423,6 +433,19 @@ public String[][] transformToStringValuesMV(ValueBlock valueBlock) {
long[][] longValuesMV = _transformFunction.transformToLongValuesMV(valueBlock);
ArrayCopyUtils.copyFromTimestamp(longValuesMV, _stringValuesMV, length);
return _stringValuesMV;
+ case UUID:
+ length = valueBlock.getNumDocs();
+ initStringValuesMV(length);
+ byte[][][] uuidValuesMV = _transformFunction.transformToBytesValuesMV(valueBlock);
+ for (int i = 0; i < length; i++) {
+ int numValues = uuidValuesMV[i].length;
+ String[] stringValues = new String[numValues];
+ for (int j = 0; j < numValues; j++) {
+ stringValues[j] = UuidUtils.toString(uuidValuesMV[i][j]);
+ }
+ _stringValuesMV[i] = stringValues;
+ }
+ return _stringValuesMV;
Review Comment:
The new MV UUID-to-STRING conversion path isn’t covered by the added tests. Please add a unit test that exercises `transformToStringValuesMV` for UUIDs (including at least one row with multiple UUIDs) to ensure canonical dashed formatting is preserved for MV inputs.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java:
##########
@@ -423,6 +433,19 @@ public String[][] transformToStringValuesMV(ValueBlock valueBlock) {
long[][] longValuesMV = _transformFunction.transformToLongValuesMV(valueBlock);
ArrayCopyUtils.copyFromTimestamp(longValuesMV, _stringValuesMV, length);
return _stringValuesMV;
+ case UUID:
+ length = valueBlock.getNumDocs();
+ initStringValuesMV(length);
+ byte[][][] uuidValuesMV = _transformFunction.transformToBytesValuesMV(valueBlock);
+ for (int i = 0; i < length; i++) {
+ int numValues = uuidValuesMV[i].length;
+ String[] stringValues = new String[numValues];
+ for (int j = 0; j < numValues; j++) {
+ stringValues[j] = UuidUtils.toString(uuidValuesMV[i][j]);
+ }
+ _stringValuesMV[i] = stringValues;
+ }
+ return _stringValuesMV;
Review Comment:
In `transformToStringValuesSV`, the `case UUID` block is wrapped in braces (`case UUID: { ... }`), but the MV version isn’t. Consider using braces here as well for consistency and to reduce the risk of scoping/maintenance issues when future edits introduce additional local variables in this case.
--
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
Re: [PR] Render UUID casts as canonical strings [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19165:
URL: https://github.com/apache/pinot/pull/19165#issuecomment-5196801602
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19165?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `80.76923%` with `5 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.12%. Comparing base ([`14333f3`](https://app.codecov.io/gh/apache/pinot/commit/14333f3f927b165caa9affa8ff656ca5aa9c86d9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`aa13d1f`](https://app.codecov.io/gh/apache/pinot/commit/aa13d1fa0705095d6f528988798c31bbeaa7e53a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/19165?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ator/transform/function/CastTransformFunction.java](https://app.codecov.io/gh/apache/pinot/pull/19165?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ftransform%2Ffunction%2FCastTransformFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQ2FzdFRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | 80.76% | [2 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (14333f3) and HEAD (aa13d1f). Click for more details.
>
> <details><summary>HEAD has 16 uploads less than BASE</summary>
>
>| Flag | BASE (14333f3) | HEAD (aa13d1f) |
>|------|------|------|
>|unittests|2|1|
>|java-25|5|1|
>|temurin|5|1|
>|unittests2|1|0|
>|integration|3|0|
>|integration1|1|0|
>|integration2|1|0|
>|custom-integration1|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19165 +/- ##
============================================
- Coverage 66.57% 57.12% -9.46%
+ Complexity 1423 1 -1422
============================================
Files 3441 2644 -797
Lines 218320 158692 -59628
Branches 34737 26073 -8664
============================================
- Hits 145341 90648 -54693
+ Misses 61251 60257 -994
+ Partials 11728 7787 -3941
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.12% <80.76%> (-9.46%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.12% <80.76%> (-9.46%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.12% <80.76%> (-9.45%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.12% <80.76%> (+<0.01%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19165?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] Render UUID casts as canonical strings [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on code in PR #19165:
URL: https://github.com/apache/pinot/pull/19165#discussion_r3723909968
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java:
##########
@@ -261,6 +262,27 @@ public String[] transformToStringValuesSV(ValueBlock valueBlock) {
ArrayCopyUtils.copyFromTimestamp(longValues, _stringValuesSV, length);
return _stringValuesSV;
}
+ case UUID: {
Review Comment:
Should the scalar cast function in `DataTypeConversionFunctions` also be updated similarly?
--
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
Re: [PR] Render UUID casts as canonical strings [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #19165:
URL: https://github.com/apache/pinot/pull/19165
--
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
Re: [PR] Render UUID casts as canonical strings [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #19165:
URL: https://github.com/apache/pinot/pull/19165#issuecomment-5211863397
Documentation follow-up: https://github.com/pinot-contrib/pinot-docs/pull/964
--
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
Re: [PR] Render UUID casts as canonical strings [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #19165:
URL: https://github.com/apache/pinot/pull/19165#discussion_r3724847803
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java:
##########
@@ -261,6 +262,27 @@ public String[] transformToStringValuesSV(ValueBlock valueBlock) {
ArrayCopyUtils.copyFromTimestamp(longValues, _stringValuesSV, length);
return _stringValuesSV;
}
+ case UUID: {
Review Comment:
No production update is needed for the scalar function: DataTypeConversionFunctions.cast identifies java.util.UUID as PinotDataType.UUID, and STRING conversion delegates to UUID.toString(), which is already canonical dashed lowercase. Added direct regression coverage in 39368242d0. Raw byte[] values remain BYTES and intentionally continue to cast as hex.
--
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
Re: [PR] Render UUID casts as canonical strings [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #19165:
URL: https://github.com/apache/pinot/pull/19165#discussion_r3723298157
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/CastTransformFunctionTest.java:
##########
@@ -35,6 +35,17 @@
public class CastTransformFunctionTest extends BaseTransformFunctionTest {
+ @Test
+ public void testCastUuidToString() {
+ String uuid = "550e8400-e29b-41d4-a716-446655440000";
+ ExpressionContext expression = RequestContextUtils.getExpression("CAST(toUuid('" + uuid + "') AS STRING)");
+ TransformFunction transformFunction = TransformFunctionFactory.get(expression, _dataSourceMap);
+
+ String[] expectedValues = new String[NUM_ROWS];
+ Arrays.fill(expectedValues, uuid);
+ testTransformFunction(transformFunction, expectedValues);
+ }
Review Comment:
Addressed in a60393d6f7: added testCastUuidMvToString, which exercises two UUID values per row and verifies canonical dashed strings.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java:
##########
@@ -423,6 +433,19 @@ public String[][] transformToStringValuesMV(ValueBlock valueBlock) {
long[][] longValuesMV = _transformFunction.transformToLongValuesMV(valueBlock);
ArrayCopyUtils.copyFromTimestamp(longValuesMV, _stringValuesMV, length);
return _stringValuesMV;
+ case UUID:
+ length = valueBlock.getNumDocs();
+ initStringValuesMV(length);
+ byte[][][] uuidValuesMV = _transformFunction.transformToBytesValuesMV(valueBlock);
+ for (int i = 0; i < length; i++) {
+ int numValues = uuidValuesMV[i].length;
+ String[] stringValues = new String[numValues];
+ for (int j = 0; j < numValues; j++) {
+ stringValues[j] = UuidUtils.toString(uuidValuesMV[i][j]);
+ }
+ _stringValuesMV[i] = stringValues;
+ }
+ return _stringValuesMV;
Review Comment:
Addressed in a60393d6f7: the new MV test calls the UUID transformToStringValuesMV path with multiple UUIDs in every row.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java:
##########
@@ -423,6 +433,19 @@ public String[][] transformToStringValuesMV(ValueBlock valueBlock) {
long[][] longValuesMV = _transformFunction.transformToLongValuesMV(valueBlock);
ArrayCopyUtils.copyFromTimestamp(longValuesMV, _stringValuesMV, length);
return _stringValuesMV;
+ case UUID:
+ length = valueBlock.getNumDocs();
+ initStringValuesMV(length);
+ byte[][][] uuidValuesMV = _transformFunction.transformToBytesValuesMV(valueBlock);
+ for (int i = 0; i < length; i++) {
+ int numValues = uuidValuesMV[i].length;
+ String[] stringValues = new String[numValues];
+ for (int j = 0; j < numValues; j++) {
+ stringValues[j] = UuidUtils.toString(uuidValuesMV[i][j]);
+ }
+ _stringValuesMV[i] = stringValues;
+ }
+ return _stringValuesMV;
Review Comment:
Addressed in aa13d1fa07: wrapped the MV UUID case in braces to match the SV case.
--
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