xiangfu0 commented on code in PR #18411:
URL: https://github.com/apache/pinot/pull/18411#discussion_r3195308148
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/FieldConfig.java:
##########
@@ -181,6 +183,8 @@ public EncodingType getEncodingType() {
@Nullable
Review Comment:
Adding `@JsonIgnore` here breaks the update-diff compatibility guarantee for
legacy `fieldConfigList[].indexType`. A create request can still come in with
`indexType`, but once the config is written back through
`JsonUtils.objectToString(fieldConfigList)` only `indexTypes` survives in ZK. A
later PUT of the same legacy payload now looks newly introduced instead of
unchanged, so the validator warns/errors on what should be a no-op update. If
we want idempotent updates for already-stored legacy configs, we need to
preserve the legacy key in stored JSON or teach the diff to treat `indexType`
and singleton `indexTypes` as equivalent.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/IndexingConfig.java:
##########
@@ -212,6 +218,7 @@ public void setLoadMode(String loadMode) {
* {@link IngestionConfig#getStreamIngestionConfig()}
Review Comment:
The rule walker only fires on getters annotated with `@DeprecatedConfig`, so
leaving `getStreamConfigs()` undecorated means the deprecated
realtime-ingestion shape is still accepted as if it were modern.
`IngestionConfigUtils` still falls back to `tableIndexConfig.streamConfigs` in
production, so users can keep creating/updating tables with this legacy path
and never see any warning/error from the new validator.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
