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/05/07 03:40:18 UTC
[PR] [codex] Expose DataSource from TransformFunction [pinot]
xiangfu0 opened a new pull request, #18435:
URL: https://github.com/apache/pinot/pull/18435
## Summary
- Add `TransformFunction#getDataSource()` so transform functions can explicitly expose a backing segment `DataSource` when one exists.
- Wire `ColumnContext.fromTransformFunction(...)` to preserve exposed data sources across transform operator chains.
- Update identifier, literal, array literal, generated array, base, and map item transform implementations to implement the new method explicitly.
- Switch `jsonExtractIndex` and `TEXT_MATCH` to consume the source from their input transform instead of re-looking up raw identifiers in `columnContextMap`.
## Why
Some index-backed transform functions need access to the segment `DataSource` for the expression they consume. Before this change, that access was only available through `ColumnContext` for raw columns, which made transform-to-transform chains lose the data source even when a transform result still mapped directly to a segment source.
## User Manual
For transform function authors:
- Implement `getDataSource()` explicitly on every `TransformFunction` implementation.
- Return the backing `DataSource` only when the transform output maps directly to a segment data source, such as an identifier or a map item key source.
- Return `null` for computed expressions and literals.
- Prefer `argument.getDataSource()` when a function needs indexes from its input expression.
## Sample Queries
These existing index-backed forms continue to work, with the data source now flowing through the transform-function API:
```sql
SELECT TEXT_MATCH(skills, 'sewing') AS match
FROM testTable;
```
```sql
SELECT jsonExtractIndex(jsonSV, '$.intVal', 'INT')
FROM testTable;
```
## Validation
- `./mvnw -pl pinot-core -am -DskipTests compile`
- `./mvnw -pl pinot-core -am -Dtest=IdentifierTransformFunctionTest,DateTimeConversionTransformFunctionTest,JsonExtractIndexTransformFunctionTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test`
- `./mvnw -pl pinot-core -am -Dtest=TextMatchTransformFunctionTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test`
--
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] Expose DataSource from TransformFunction [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #18435:
URL: https://github.com/apache/pinot/pull/18435#discussion_r3199091302
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunction.java:
##########
@@ -79,6 +80,13 @@ default void init(List<TransformFunction> arguments, Map<String, ColumnContext>
@Nullable
Dictionary getDictionary();
+ /**
+ * Returns the data source backing the transform result when the result maps directly to a segment data source, or
+ * {@code null} otherwise.
+ */
+ @Nullable
+ DataSource getDataSource();
Review Comment:
Adding a new abstract method to the TransformFunction interface is a binary-incompatible change for any custom/third-party TransformFunction implementations (e.g., those registered via TransformFunctionFactory.init from server config). Older implementations will throw AbstractMethodError when ColumnContext.fromTransformFunction/TransformOperator calls getDataSource(). Consider making getDataSource() a default method that returns null to preserve backward compatibility, while still encouraging overrides for functions that can expose a backing DataSource.
--
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] Expose DataSource from TransformFunction [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #18435:
URL: https://github.com/apache/pinot/pull/18435#discussion_r3198845608
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractIndexTransformFunction.java:
##########
@@ -70,22 +71,20 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
}
TransformFunction firstArgument = arguments.get(0);
- if (firstArgument instanceof IdentifierTransformFunction) {
- String columnName = ((IdentifierTransformFunction) firstArgument).getColumnName();
- _jsonIndexReader = columnContextMap.get(columnName).getDataSource().getJsonIndex();
- if (_jsonIndexReader == null) { //TODO: rework
- Optional<IndexType<?, ?, ?>> compositeIndex =
- IndexService.getInstance().getOptional("composite_json_index");
+ DataSource dataSource = firstArgument.getDataSource();
+ if (dataSource != null) {
+ _jsonIndexReader = dataSource.getJsonIndex();
+ if (_jsonIndexReader == null) { // TODO: rework
+ Optional<IndexType<?, ?, ?>> compositeIndex = IndexService.getInstance().getOptional("composite_json_index");
if (compositeIndex.isPresent()) {
- _jsonIndexReader = (JsonIndexReader) columnContextMap.get(columnName)
- .getDataSource().getIndex(compositeIndex.get());
+ _jsonIndexReader = (JsonIndexReader) dataSource.getIndex(compositeIndex.get());
}
}
if (_jsonIndexReader == null) {
throw new IllegalStateException("jsonExtractIndex can only be applied on a column with JSON index");
}
} else {
- throw new IllegalArgumentException("jsonExtractIndex can only be applied to a raw column");
+ throw new IllegalArgumentException("jsonExtractIndex can only be applied to a column with JSON index");
Review Comment:
The `dataSource == null` branch now throws the same high-level message as the \"no JSON index\" path, but the actionable fix differs (expose/provide a `DataSource` vs. add/build an index). Consider changing the `IllegalArgumentException` message to explicitly indicate that the first argument must be backed by a segment `DataSource` (i.e., an expression that can expose a source), while keeping the existing message for the case where a source exists but lacks a JSON index.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunction.java:
##########
@@ -79,6 +80,13 @@ default void init(List<TransformFunction> arguments, Map<String, ColumnContext>
@Nullable
Dictionary getDictionary();
+ /**
+ * Returns the data source backing the transform result when the result maps directly to a segment data source, or
+ * {@code null} otherwise.
+ */
+ @Nullable
+ DataSource getDataSource();
Review Comment:
Adding a new abstract method to a public interface is a binary-incompatible change: existing custom/third-party `TransformFunction` implementations compiled against older versions will fail to load at runtime (e.g., `AbstractMethodError`). Consider making this a `default` method returning `null` to preserve binary compatibility while still allowing implementations to override and expose a backing `DataSource`.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TextMatchTransformFunction.java:
##########
@@ -50,15 +50,12 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
super.init(arguments, columnContextMap);
TransformFunction columnArg = arguments.get(0);
- if (!(columnArg instanceof IdentifierTransformFunction && columnArg.getResultMetadata().isSingleValue())) {
+ DataSource dataSource = columnArg.getDataSource();
+ if (!columnArg.getResultMetadata().isSingleValue() || dataSource == null) {
throw new IllegalArgumentException(
"The first argument of TEXT_MATCH transform function must be a single-valued column");
}
Review Comment:
The new guard conflates two different failure modes (multi-value input vs. missing `DataSource`) but always reports \"must be a single-valued column\". When `isSingleValue()` is true but `dataSource == null` (e.g., computed expression or a transform that didn’t expose its source), this message is inaccurate. Split the checks and provide a specific message for the `dataSource == null` case (e.g., requiring a segment-backed source with a text index).
--
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] Expose DataSource from TransformFunction [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18435:
URL: https://github.com/apache/pinot/pull/18435#issuecomment-4394247111
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18435?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 `61.90476%` with `8 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 63.58%. Comparing base ([`22df831`](https://app.codecov.io/gh/apache/pinot/commit/22df83145b767f807794426faf7ebcd40ab1af33?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1a336cb`](https://app.codecov.io/gh/apache/pinot/commit/1a336cb98b577ebeffb6f470615071c0ff554887?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 67 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/18435?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/ItemTransformFunction.java](https://app.codecov.io/gh/apache/pinot/pull/18435?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ftransform%2Ffunction%2FItemTransformFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXRlbVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18435?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...rm/function/JsonExtractIndexTransformFunction.java](https://app.codecov.io/gh/apache/pinot/pull/18435?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ftransform%2Ffunction%2FJsonExtractIndexTransformFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSnNvbkV4dHJhY3RJbmRleFRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | 57.14% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18435?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...nsform/function/ArrayLiteralTransformFunction.java](https://app.codecov.io/gh/apache/pinot/pull/18435?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ftransform%2Ffunction%2FArrayLiteralTransformFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQXJyYXlMaXRlcmFsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18435?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...sform/function/GenerateArrayTransformFunction.java](https://app.codecov.io/gh/apache/pinot/pull/18435?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ftransform%2Ffunction%2FGenerateArrayTransformFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vR2VuZXJhdGVBcnJheVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18435?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18435 +/- ##
============================================
+ Coverage 63.40% 63.58% +0.17%
- Complexity 1668 1717 +49
============================================
Files 3252 3252
Lines 198661 199138 +477
Branches 30770 30875 +105
============================================
+ Hits 125965 126620 +655
+ Misses 62632 62441 -191
- Partials 10064 10077 +13
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18435/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/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.58% <61.90%> (+0.17%)` | :arrow_up: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.58% <61.90%> (+0.17%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.58% <61.90%> (+0.17%)` | :arrow_up: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.67% <61.90%> (+0.29%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.90% <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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/18435?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] Expose DataSource from TransformFunction [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 closed pull request #18435: Expose DataSource from TransformFunction
URL: https://github.com/apache/pinot/pull/18435
--
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