You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "deepthi912 (via GitHub)" <gi...@apache.org> on 2025/10/18 03:02:11 UTC
[PR] Add a framework to validate each of the Ingestion Transformation functions [pinot]
deepthi912 opened a new pull request, #17039:
URL: https://github.com/apache/pinot/pull/17039
This PR adds a validation framework for Pinot transform functions used in ingestion configs. It provides validation hooks for datatype checks in the TransformFunction interface that individual functions can implement to validate their configurations during table creation.
We can include validationMode in the transform function specification:
1. **LEGACY** Mode (Default)
Purpose: Backward compatibility - allows all existing type conversions
Behavior: No validation, accepts everything (current Pinot behavior)
Use Case: Existing tables that shouldn't break
2. **LENIENT** Mode (Recommended)
Purpose: Safe type conversions allowed
Behavior: Allows safe conversions like INT→LONG, FLOAT→DOUBLE, but blocks unsafe ones like STRING→INT
Use Case: New tables that want some safety but flexibility
3. **STRICT** Mode
Purpose: Maximum type safety
Behavior: No automatic type conversions, exact type matching required
Use Case: Critical tables where type safety is paramount
```
{
"columnName": "processed_age",
"transformFunction": "CAST(age_string AS INT)",
"validationMode": "STRICT"
}
```
--
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] Add a framework to validate each of the Ingestion Transformation functions [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17039:
URL: https://github.com/apache/pinot/pull/17039#issuecomment-3417770191
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17039?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 `47.36842%` with `10 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 56.24%. Comparing base ([`1b6866d`](https://app.codecov.io/gh/apache/pinot/commit/1b6866d5aea791c5b8f93f5b397e284047797a61?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`3b4410f`](https://app.codecov.io/gh/apache/pinot/commit/3b4410f0b8c1eebde7c502fb6f1550e8f51c4053?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 131 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/17039?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...operator/transform/function/TransformFunction.java](https://app.codecov.io/gh/apache/pinot/pull/17039?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ftransform%2Ffunction%2FTransformFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ot/spi/config/table/ingestion/IngestionConfig.java](https://app.codecov.io/gh/apache/pinot/pull/17039?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Fconfig%2Ftable%2Fingestion%2FIngestionConfig.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL2luZ2VzdGlvbi9Jbmdlc3Rpb25Db25maWcuamF2YQ==) | 33.33% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ot/spi/config/table/ingestion/TransformConfig.java](https://app.codecov.io/gh/apache/pinot/pull/17039?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Fconfig%2Ftable%2Fingestion%2FTransformConfig.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL2luZ2VzdGlvbi9UcmFuc2Zvcm1Db25maWcuamF2YQ==) | 88.88% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17039?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 #17039 +/- ##
============================================
- Coverage 56.42% 56.24% -0.18%
+ Complexity 702 701 -1
============================================
Files 2406 2416 +10
Lines 133681 135041 +1360
Branches 21260 21566 +306
============================================
+ Hits 75424 75958 +534
- Misses 51983 52717 +734
- Partials 6274 6366 +92
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17039/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/17039/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/17039/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/17039/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/17039/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.24% <47.36%> (-0.15%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17039/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.24% <47.36%> (-0.18%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17039/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.24% <47.36%> (-0.18%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17039/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.24% <47.36%> (-0.18%)` | :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/17039?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] Add a framework to validate each of the Ingestion Transformation functions [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on code in PR #17039:
URL: https://github.com/apache/pinot/pull/17039#discussion_r2524880724
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunction.java:
##########
@@ -171,4 +172,55 @@ default void init(List<TransformFunction> arguments, Map<String, ColumnContext>
*/
@Nullable
RoaringBitmap getNullBitmap(ValueBlock block);
+
+ /**
+ * Validates transform function configuration during table creation.
+ */
+ default void validateIngestionConfig(String transformFunctionExpression, org.apache.pinot.spi.data.Schema schema) {
+ // Default: no validation
+ }
+
+ /**
+ * Returns whether this function supports ingestion-time transformation.
+ */
+ default boolean supportsIngestionTransform() {
Review Comment:
Yeah +1, if this is for ingestion transforms specifically, it doesn't make sense to put this here because `TransformFunction`s can never be used for ingestion side things like transforms (despite the name), filtering etc.
Although I'm not sure I entirely understand the goal of this PR, could you please elaborate on the exact use 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
Re: [PR] Add a framework to validate each of the Ingestion Transformation functions [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #17039:
URL: https://github.com/apache/pinot/pull/17039#discussion_r2524849682
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunction.java:
##########
@@ -171,4 +172,55 @@ default void init(List<TransformFunction> arguments, Map<String, ColumnContext>
*/
@Nullable
RoaringBitmap getNullBitmap(ValueBlock block);
+
+ /**
+ * Validates transform function configuration during table creation.
+ */
+ default void validateIngestionConfig(String transformFunctionExpression, org.apache.pinot.spi.data.Schema schema) {
+ // Default: no validation
+ }
+
+ /**
+ * Returns whether this function supports ingestion-time transformation.
+ */
+ default boolean supportsIngestionTransform() {
Review Comment:
Ingestion transform is not using `TransformFunction`, but scalar function. I feel the framework should be built on scalar function, or `FunctionRegistry`
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunction.java:
##########
@@ -171,4 +172,55 @@ default void init(List<TransformFunction> arguments, Map<String, ColumnContext>
*/
@Nullable
RoaringBitmap getNullBitmap(ValueBlock block);
+
+ /**
+ * Validates transform function configuration during table creation.
+ */
+ default void validateIngestionConfig(String transformFunctionExpression, org.apache.pinot.spi.data.Schema schema) {
Review Comment:
Replace with proper import, same for other places
--
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