Skip to content

Annotation-driven deprecated table config validation on create + update#18411

Open
xiangfu0 wants to merge 16 commits intoapache:masterfrom
xiangfu0:validate-deprecated-table-config-create
Open

Annotation-driven deprecated table config validation on create + update#18411
xiangfu0 wants to merge 16 commits intoapache:masterfrom
xiangfu0:validate-deprecated-table-config-create

Conversation

@xiangfu0
Copy link
Copy Markdown
Contributor

@xiangfu0 xiangfu0 commented May 4, 2026

Summary

Reject explicit use of deprecated table-config keys on both create and update, driven by a single source of truth on the SPI getters instead of a hand-maintained rule list.

  • New @DeprecatedConfig(replacement, since) annotation in pinot-spi. Placed on the deprecated getter; since is the Pinot release the field was first marked @Deprecated.
  • DeprecatedTableConfigValidationUtils discovers rules at class-load via a Jackson-aware reflection walk over TableConfig (recursing into nested BaseJsonConfig types, Collection<X>, Map<?,V>, honoring @JsonProperty renames). The hand-maintained list is gone.
  • Severity is version-aware: a rule whose since.major.minor equals the running PinotVersion.major.minor is reported as a warning (one-release grace period). Older rules are errors that block the request. Unknown current version → safe default of error.
  • Update paths (PUT /tables/{name}, PUT /tableConfigs/{name}, validate POSTs against existing tables) diff the incoming JSON against the byte-faithful stored ZNRecord JSON (new helper TableConfigSerDeUtils.toRawJsonNode + ZKMetadataProvider.getTableConfigZNRecord) — not against existingConfig.toJsonNode(), which would silently strip @JsonIgnore-d / @JsonInclude(NON_DEFAULT) deprecated keys and turn every legacy PUT into a false positive. Re-submitting an unchanged legacy value is a no-op; only newly introduced or value-changed deprecated paths fire.
  • Warnings surface via a new optional deprecationWarnings: List<String> field on ConfigSuccessResponse and the validate endpoint JSON. Errors continue to throw 400.

Deprecated table-config keys covered

Sorted from earliest deprecation to latest. On the current 1.6.0-SNAPSHOT release line, everything older than 1.6 is an error; 1.6.0 deprecations are warnings (one-release grace period).

Since JSON path Replacement Severity on 1.6
0.3.0 routing.routingTableBuilderName Use routing.segmentPrunerTypes and routing.instanceSelectorType error
0.7.1 tableIndexConfig.streamConfigs Use ingestionConfig.streamIngestionConfig.streamConfigMaps error
0.8.0 segmentsConfig.segmentPushFrequency Use ingestionConfig.batchIngestionConfig.segmentIngestionFrequency error
0.8.0 segmentsConfig.segmentPushType Use ingestionConfig.batchIngestionConfig.segmentIngestionType error
0.9.0 fieldConfigList[*].indexType Use fieldConfigList[].indexTypes error
0.12.0 tableIndexConfig.jsonIndexColumns Use tableIndexConfig.jsonIndexConfigs error
1.1.0 segmentsConfig.replicasPerPartition Use segmentsConfig.replication error
1.1.0 instanceAssignmentConfigMap[*].replicaGroupPartitionConfig.minimizeDataMovement Remove this field; it will be removed in a future release error
1.3.0 segmentsConfig.replicaGroupStrategyConfig Use segmentAssignmentConfigMap error
1.3.0 segmentsConfig.minimizeDataMovement Use instanceAssignmentConfigMap error
1.4.0 upsertConfig.enableSnapshot Use upsertConfig.snapshot error
1.4.0 upsertConfig.enablePreload Use upsertConfig.preload error
1.4.0 upsertConfig.allowPartialUpsertConsumptionDuringCommit Use ingestionConfig.streamIngestionConfig.parallelSegmentConsumptionPolicy error
1.4.0 dedupConfig.enablePreload Use dedupConfig.preload error
1.4.0 dedupConfig.allowDedupConsumptionDuringCommit Use ingestionConfig.streamIngestionConfig.parallelSegmentConsumptionPolicy error
1.6.0 tableIndexConfig.createInvertedIndexDuringSegmentGeneration Remove this field; it is ignored warning

since was determined per field by walking git log / git tag --contains against the upstream apache/pinot history to find the first release tag that ships the @Deprecated annotation (or the original @deprecated Javadoc when that came first).

Behavior

  • Create (POST): every present rule fires. Errors → 400; warnings → server WARN log + deprecationWarnings field.
  • Update (PUT): diff against the raw stored ZK JSON. Only paths newly added or whose value changed fire. Legacy values that were already on the table and re-submitted unchanged pass silently.
  • Validate (POST): runs in create mode if the table doesn't yet exist, update mode otherwise. Returns warnings in the response body.
  • Builder-generated payloads: TableConfigBuilder.build() now converts the deprecated _segmentPushType / _segmentPushFrequency setters into modern ingestionConfig.batchIngestionConfig.segmentIngestionType/Frequency, so existing tests and tools that use the builder produce create payloads that pass validation.

Testing

  • ./mvnw -pl pinot-spi,pinot-common,pinot-controller -am -Dtest='DeprecatedTableConfigValidationUtilsTest+TableConfigSerDeUtilsTest+TableConfigBuilderTest+PinotTableRestletResourceTest#testRejectsDeprecatedConfigOnCreateAndOnUpdateWhenNewlyIntroduced+PinotTableRestletResourceTest#testUpdateAllowsUnchangedLegacyDeprecatedConfig+TableConfigsRestletResourceTest' test
  • ./mvnw -pl pinot-spi,pinot-common,pinot-controller spotless:apply checkstyle:check license:check

Coverage includes: annotation discovery walk against TableConfig, diff filtering on update, version-based severity classification, raw-JSON preservation across @JsonIgnore/@JsonInclude(NON_DEFAULT) getters, and round-trip rejection/acceptance through the controller REST endpoints.

@xiangfu0 xiangfu0 force-pushed the validate-deprecated-table-config-create branch from 327db64 to e02ba1c Compare May 4, 2026 06:07
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 4, 2026

Codecov Report

❌ Patch coverage is 66.37681% with 116 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.64%. Comparing base (642f07d) to head (e252bed).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
...esources/DeprecatedTableConfigValidationUtils.java 67.88% 29 Missing and 41 partials ⚠️
...ler/api/resources/TableConfigsRestletResource.java 65.62% 20 Missing and 2 partials ⚠️
...oller/api/resources/PinotTableRestletResource.java 51.28% 18 Missing and 1 partial ⚠️
...he/pinot/spi/utils/builder/TableConfigBuilder.java 0.00% 2 Missing and 1 partial ⚠️
...not/common/utils/config/TableConfigSerDeUtils.java 86.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18411      +/-   ##
============================================
+ Coverage     63.55%   63.64%   +0.09%     
- Complexity     1717     1735      +18     
============================================
  Files          3252     3255       +3     
  Lines        199051   199769     +718     
  Branches      30838    31059     +221     
============================================
+ Hits         126503   127152     +649     
+ Misses        62471    62436      -35     
- Partials      10077    10181     +104     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 63.64% <66.37%> (+0.09%) ⬆️
temurin 63.64% <66.37%> (+0.09%) ⬆️
unittests 63.64% <66.37%> (+0.09%) ⬆️
unittests1 55.69% <68.42%> (+0.06%) ⬆️
unittests2 35.01% <66.37%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0 xiangfu0 added configuration Config changes (addition/deletion/change in behavior) deprecation Marks deprecated APIs, configs, or features labels May 4, 2026
Copy link
Copy Markdown
Contributor

@noob-se7en noob-se7en left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 major error-handling / 2 medium follow-ups.

@xiangfu0 xiangfu0 force-pushed the validate-deprecated-table-config-create branch from 2fae407 to fbc259a Compare May 4, 2026 22:28
@xiangfu0 xiangfu0 changed the title Reject deprecated table configs on create Annotation-driven deprecated table config validation on create + update May 4, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces annotation-driven validation to reject explicit usage of deprecated TableConfig JSON keys on both table/config creation and updates, while preserving backward compatibility for legacy values already stored in ZK by diffing against the raw stored ZNRecord JSON.

Changes:

  • Add @DeprecatedConfig(replacement, since) annotations on deprecated SPI config getters and reflectively discover deprecated JSON paths for validation.
  • Enforce deprecated-config validation on controller create/update/validate endpoints (with version-aware warning vs error severity) and surface warnings via deprecationWarnings.
  • Update builders, tests, and example table-config JSON to use modern fields (ingestion configs, indexTypes, jsonIndexConfigs, etc.), plus add raw-ZK JSON reconstruction utilities for update diffing.

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
pinot-spi/src/main/java/org/apache/pinot/spi/config/DeprecatedConfig.java Adds the new deprecation annotation used as the single source of truth for deprecated JSON keys.
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/DeprecatedTableConfigValidationUtils.java Implements reflective rule discovery + create/update-time deprecated-key validation with version-aware severity.
pinot-common/src/main/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtils.java Adds toRawJsonNode(ZNRecord) to reconstruct raw stored JSON for byte-faithful update diffing.
pinot-common/src/main/java/org/apache/pinot/common/metadata/ZKMetadataProvider.java Adds getTableConfigZNRecord() helper to fetch raw table config ZNRecord.
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java Enforces deprecated-config validation on table create/update/validate and returns warnings in responses.
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java Enforces deprecated-config validation for TableConfigs create/update/validate and returns warnings in responses.
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ConfigSuccessResponse.java Adds optional deprecationWarnings field to success responses.
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java Converts deprecated builder setters (segment push + stream configs) into modern ingestion config fields and omits deprecated serialized keys.
pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableConfigBuilderTest.java Tests conversion/omission behavior of deprecated fields in builder output JSON.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/SegmentsValidationAndRetentionConfig.java Annotates deprecated segment push + other deprecated fields with @DeprecatedConfig.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/IndexingConfig.java Annotates deprecated indexing fields (jsonIndexColumns, streamConfigs, etc.) with @DeprecatedConfig.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/FieldConfig.java Marks legacy indexType as deprecated config key for validation while preserving deserialization via @JsonCreator.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/RoutingConfig.java Annotates deprecated routingTableBuilderName with @DeprecatedConfig.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/UpsertConfig.java Annotates deprecated upsert booleans with @DeprecatedConfig and NON_DEFAULT inclusion.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/DedupConfig.java Annotates deprecated dedup booleans with @DeprecatedConfig and NON_DEFAULT inclusion.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/assignment/InstanceReplicaGroupPartitionConfig.java Annotates deprecated nested minimizeDataMovement with @DeprecatedConfig.
pinot-common/src/test/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtilsTest.java Adds tests ensuring toRawJsonNode() preserves keys stripped by bean round-trips.
pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/DeprecatedTableConfigValidationUtilsTest.java Adds unit tests for rule discovery, version severity, create vs update diff behavior.
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java Adds REST-level tests for rejecting deprecated keys on create and on update when newly introduced.
pinot-controller/src/test/java/org/apache/pinot/controller/api/TableConfigsRestletResourceTest.java Adds TableConfigs REST test ensuring deprecated keys are rejected on create.
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancerClusterStatelessTest.java Updates test table config creation to use modern ingestion config handling.
pinot-core/src/test/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManagerTest.java Migrates tests to use ingestion-config helpers instead of deprecated streamConfigs.
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BasePauselessRealtimeIngestionTest.java Migrates ingestion setup away from deprecated streamConfigs.
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/logicaltable/LogicalTableWithTwoRealtimeTableIntegrationTest.java Migrates stream config access to ingestion-config utilities.
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/realtime/ingestion/KafkaIncreaseDecreasePartitionsIntegrationTest.java Refactors test to rely on base-class topic/table wiring rather than manual creation.
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java Adjusts legacy conversion test to inject deprecated keys directly (since builder now produces modern ingestion config).
pinot-tools/src/main/java/org/apache/pinot/tools/BootstrapTableTool.java Adds a null-guard around batch config maps iteration.
pinot-tools/src/main/resources/conf/sample_offline_table_config.json Updates sample config to modern ingestion fields / removes deprecated keys.
pinot-tools/src/main/resources/conf/sample_realtime_table_config.json Updates sample config to modern ingestion fields / removes deprecated keys.
pinot-tools/src/main/resources/examples/batch/airlineStats/airlineStats_offline_table_config.json Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/baseballStats/baseballStats_offline_table_config.json Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/minions/batch/baseballStats/baseballStats_offline_table_config.json Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/clickstreamFunnel/clickstreamFunnel_offline_table_config.json Removes deprecated keys and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/dimBaseballTeams/dimBaseballTeams_offline_table_config.json Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/fineFoodReviews/fineFoodReviews_offline_table_config.json Migrates field configs to indexTypes and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/githubEvents/githubEvents_offline_table_config.json Migrates jsonIndexColumns -> jsonIndexConfigs and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/githubComplexTypeEvents/githubComplexTypeEvents_offline_table_config.json Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/starbucksStores/starbucksStores_offline_table_config.json Migrates H3 field config to indexTypes and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/testUnnest/testUnnest_offline_table_config.json Removes deprecated fields and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/dailySales/dailySales_realtime_table_config.json Removes deprecated keys and migrates to modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/fineFoodReviews/fineFoodReviews_realtime_table_config.json Removes deprecated keys and migrates to modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/fineFoodReviews_part_0/fineFoodReviews_part_0_realtime_table_config.json Removes deprecated keys and migrates to modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/fineFoodReviews_part_1/fineFoodReviews_part_1_realtime_table_config.json Removes deprecated keys and migrates to modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/meetupRsvpJson/meetupRsvpJson_realtime_table_config.json Migrates jsonIndexColumns -> jsonIndexConfigs.
pinot-tools/src/main/resources/examples/stream/upsertJsonMeetupRsvp/upsertJsonMeetupRsvp_realtime_table_config.json Migrates jsonIndexColumns -> jsonIndexConfigs.
pinot-tools/src/main/resources/examples/stream/upsertMeetupRsvp/upsertMeetupRsvp_realtime_table_config.json Migrates upsert deprecated fields to modern enums; migrates indexType -> indexTypes.
pinot-tools/src/main/resources/examples/stream/upsertPartialMeetupRsvp/upsertPartialMeetupRsvp_realtime_table_config.json Migrates indexType -> indexTypes.
pinot-integration-tests/src/test/resources/chaos-monkey-create-table.json Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-integration-tests/src/test/resources/dimDayOfWeek_config.json Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/admin/README.md Documents modern create payload fields and common migrations away from deprecated keys.
Comments suppressed due to low confidence (1)

pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:238

  • setSegmentPushType() / setSegmentPushFrequency() are documented as deprecated in their Javadoc, but they are not annotated with @Deprecated (unlike other deprecated builder APIs in this class, e.g. setLLC).

Annotate these methods with @Deprecated so callers get compiler warnings and IDE tooling consistently flags their use.


  /**
   * @deprecated Use {@code segmentIngestionType} from {@link IngestionConfig#getBatchIngestionConfig()}
   */
  public TableConfigBuilder setSegmentPushType(String segmentPushType) {
    if (REFRESH_SEGMENT_PUSH_TYPE.equalsIgnoreCase(segmentPushType)) {
      _segmentPushType = REFRESH_SEGMENT_PUSH_TYPE;
    } else {
      _segmentPushType = "APPEND";
    }
    return this;
  }

  /**
   * @deprecated Use {@code segmentIngestionFrequency} from {@link IngestionConfig#getBatchIngestionConfig()}
   */
  public TableConfigBuilder setSegmentPushFrequency(String segmentPushFrequency) {
    _segmentPushFrequency = segmentPushFrequency;
    return this;
  }

Comment thread pinot-spi/src/main/java/org/apache/pinot/spi/config/DeprecatedConfig.java Outdated
@xiangfu0 xiangfu0 requested a review from noob-se7en May 4, 2026 23:28
xiangfu0 added 10 commits May 5, 2026 02:30
…re severity

Introduce @DeprecatedConfig(replacement, since) on SPI getters and replace
the manually-maintained rule list in DeprecatedTableConfigValidationUtils
with a Jackson-aware reflection walk over TableConfig. Severity is decided
by comparing the annotation since against PinotVersion.VERSION major.minor
so deprecations from the current release line are warnings and older ones
are errors.

Wire both create and update paths to validate against deprecated keys.
Update paths diff incoming JSON against the byte-faithful stored ZNRecord
JSON via the new TableConfigSerDeUtils.toRawJsonNode + getTableConfigZNRecord
so legacy values left over on existing tables don't trigger false-positive
"newly introduced" errors on every PUT. Surface non-fatal warnings via
ConfigSuccessResponse.deprecationWarnings.
- Drop String.format and full request body from "Invalid TableConfigs"
  error messages; match the pattern used by addConfig (per apache#14404).
- Parameterize DeprecatedTableConfigValidationUtilsTest with a
  @dataProvider over every rule discovered by the annotation walk so
  test coverage tracks the rule list 1:1.
- copyTable: catch IllegalArgumentException (not IllegalStateException)
  to keep deprecated-config rejection at 400 instead of falling through
  to the generic 500 handler.
- DeprecatedTableConfigValidationUtils Javadoc: align documented exception
  type with the actual IllegalArgumentException thrown by validateOnCreate
  / validateOnUpdate.
- @DeprecatedConfig: restrict @target to METHOD only, since the discovery
  walk only inspects methods. Prevents misleading field-level annotations
  that would silently never fire.
- Stream realtime example configs: change segmentAssignmentConfigMap key
  from REALTIME to COMPLETED. The map is keyed by assignment type, not
  table type; SegmentAssignmentStrategyFactory looks it up via
  assignmentType.toUpperCase() so REALTIME entries were dead.
setSegmentPushType, setSegmentPushFrequency, and setStreamConfigs in
TableConfigBuilder were documented as deprecated in their Javadoc but
not annotated, so callers got no compiler warning. Add @deprecated and
convert the comments to /// Markdown style for consistency.
DeprecatedTableConfigValidationUtils:
- Scoped isJacksonDefault skip to apply only when stored value is absent
  (oldValue == null) so a deliberate flip of an existing non-default
  value back to the default is still flagged.
- Tightened isJacksonDefault numeric checks: short/int/long via
  longValue(); BigInteger / BigDecimal via dedicated zero comparisons;
  floating-point via Double.compare so -0.0 is non-default. Avoids the
  lossy double coercion of arbitrary-precision types.
- Tightened textual branch to match Jackson NON_DEFAULT: only explicit
  null is default for null-default String fields; empty string is a
  real user value and must be flagged on update.
- Added classifySeverity(String, String) test seam so the version-
  unknown WARNING fallback can be unit-tested.
- Updated class-level Javadoc to describe the WARNING fallback when the
  running Pinot version is unknown (was previously documented as ERROR).

PinotTableRestletResource / TableConfigsRestletResource:
- Split BAD_REQUEST catches so ZK transient failures (RuntimeException
  from getTableConfigZNRecord / validate helpers) propagate as 5xx
  instead of being mis-reported as 400.
- Added warn-log + ControllerMeter increment on each new RuntimeException
  catch so on-call observability is preserved.
- Capture JsonNode view of the request body once on update path to avoid
  double-parse before the deprecation diff.
- Removed dead uppercase fallback in subConfigJson; added Javadoc
  pointing to the deserialization invariant it relies on.
- Removed unreleased legacy validateNoDeprecatedConfigs(JsonNode[, String])
  shims; one remaining caller (copyTable) inlined to validateOnCreate.

ConfigSuccessResponse:
- Moved @JsonInclude(NON_EMPTY) from field to getter so empty
  deprecationWarnings are correctly elided from the wire.
- Normalised null _unrecognizedProperties to Map.of().

@DeprecatedConfig SPI annotation:
- Restricted @target to METHOD only (matches discovery).
- Added "metadata-only" Javadoc note clarifying that the annotation has
  no runtime serialization effect.

Tests:
- Parameterized DeprecatedTableConfigValidationUtilsTest over discovered
  rules, exercising both array-wildcard and map-wildcard JSON shapes.
- Added regression coverage for the new isJacksonDefault skip semantics
  (re-submitted default, deliberate flip, explicit JSON null on stored
  side, empty string on textual paths).
- Added testSeverityFallsBackToWarningWhenCurrentVersionUnknown +
  testSeverityWithExplicitCurrentVersion locking the WARNING fallback.
- Added ConfigSuccessResponseTest for NON_EMPTY behavior + null
  normalisation.
- Added testUppercaseOfflineRealtimeKeysAreIgnoredByJackson in
  TableConfigsSerializationTest locking the invariant subConfigJson
  relies on.

Quickstart configs (4 stream realtime examples): segmentAssignmentConfigMap
key changed from REALTIME to COMPLETED (the map is keyed by assignment
type, not table type; SegmentAssignmentStrategyFactory looks it up via
assignmentType.toUpperCase() so REALTIME entries were dead).
@xiangfu0 xiangfu0 force-pushed the validate-deprecated-table-config-create branch from 5870292 to 3cb2910 Compare May 5, 2026 09:31
xiangfu0 and others added 5 commits May 5, 2026 02:52
PUT to a nonexistent table whose body included a deprecated key was
returning a misleading 400 ("Newly introduced deprecated table config
properties are not allowed: ...") because the deprecation diff ran with
oldTableConfigJson==null (which fell through to create-mode) before
the hasTable() check.

- PinotTableRestletResource.updateTableConfig: hasTable check moved
  above the deprecation diff so missing table reports 404 cleanly.
- TableConfigsRestletResource.updateConfig: symmetric reorder so
  missing table reports the existing "does not exist" 400 instead of
  a deprecation 400.
- DeprecatedTableConfigValidationUtils.validateOnUpdate: tightened
  contract to reject null oldTableConfigJson so callers cannot
  accidentally exercise create-mode under update semantics.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- PinotTableRestletResourceTest.testUpdateMissingTableReports404NotDeprecationError:
  regression for the recent existence-check reorder; PUT to a missing
  table whose body has a deprecated key now reports 404 cleanly.
- TableConfigsRestletResourceTest.testUpdateConfigRejectsNewlyIntroducedDeprecatedProperty:
  symmetric to the create-mode rejection, asserts a PUT that introduces
  a new deprecated key on an existing TableConfigs returns 400 with the
  deprecated path in the message.
- TableConfigsRestletResourceTest.testUpdateMissingTableConfigsReportsNotExistsNotDeprecation:
  regression for the symmetric reorder in TableConfigsRestletResource;
  PUT to a missing TableConfigs reports "does not exist" instead of a
  misleading deprecation 400.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uency

CI surfaced a wider issue with the TableConfigBuilder rewrite that was
bundled with this PR: ~14 realtime integration tests rely on reading
indexingConfig.streamConfigs after building a config via the builder, so
removing the legacy population from build() broke their setUp().

Rather than migrate every test to read from
ingestionConfig.streamIngestionConfig in this PR, remove the three
deprecation annotations whose enforcement requires the builder rewrite,
and revert TableConfigBuilder.build() to its prior shape that populates
the legacy fields. The remaining @DeprecatedConfig annotations
(replicasPerPartition, jsonIndexColumns, indexType, etc.) still validate
on create + update via the diff-based version-aware path.

The dropped deprecations can ship in a follow-up PR after the codebase
migrates fully to the ingestionConfig shape.

- pinot-spi: drop @DeprecatedConfig from getStreamConfigs(),
  getSegmentPushType(), getSegmentPushFrequency()
- pinot-spi: revert TableConfigBuilder.build() to populate
  validationConfig.{segmentPushType,segmentPushFrequency} and
  indexingConfig.streamConfigs
- pinot-controller tests: migrate path-specific assertions from
  segmentPushType/streamConfigs to replicasPerPartition (still
  deprecated since 1.1.0)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
testBuildConvertsDeprecatedIngestionFields was added when the builder
auto-routed legacy ingestion fields into ingestionConfig. The previous
commit reverted that rewrite, so the test now contradicts the reverted
behavior. testBuildOmitsDefaultDeprecatedFieldsFromSerializedJson
remains valid because it depends on @JsonInclude(NON_DEFAULT) on the
beans, not on the builder's ingestion routing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CI failure surface revealed that Pinot's own integration tests
(CommitTimeCompactionIntegrationTest, dedup/upsert tests, controller
periodic tasks) actively use deprecated table-config setters
(setReplicaGroupStrategyConfig, setReplicasPerPartition, etc.). Hard
ERROR rejection on create blocks every one of those tests with 400.

Demote classifySeverity so every parseable @DeprecatedConfig.since
returns WARNING. Only an unparseable since (a code-side annotation
bug) remains ERROR. The validator still surfaces every deprecated key
as a warning on both create and update — just no longer rejects.

Hard ERROR enforcement (the originally-designed older-than-current
threshold) can ship in a follow-up PR after the test base and
TableConfigBuilder migrate off the deprecated paths.

- DeprecatedTableConfigValidationUtils.classifySeverity: drop
  current-major-minor comparison; always return WARNING for parseable
  since
- DeprecatedTableConfigValidationUtilsTest: rewrite tests asserting
  ERROR semantics to assert WARNING semantics; consolidate the two
  current-version tests into one soft-launch test
- PinotTableRestletResourceTest.testReportsDeprecatedConfigOnCreate-
  AndOnUpdateAsWarning: assert deprecationWarnings field on success
  response instead of 400 rejection
- TableConfigsRestletResourceTest: same shape change for the create
  + update tests on /tableConfigs

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@xiangfu0 xiangfu0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found two high-signal deprecation-validation gaps; see inline comments.

Comment thread pinot-spi/src/main/java/org/apache/pinot/spi/config/table/IndexingConfig.java Outdated
@xiangfu0 xiangfu0 force-pushed the validate-deprecated-table-config-create branch 2 times, most recently from 3558fc1 to 1173e98 Compare May 7, 2026 07:34
Copy link
Copy Markdown
Contributor Author

@xiangfu0 xiangfu0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one high-signal enforcement gap; see inline comment.

@xiangfu0 xiangfu0 force-pushed the validate-deprecated-table-config-create branch from 1173e98 to a4a4b3f Compare May 8, 2026 01:25
Restore the streamConfigs @DeprecatedConfig rule now that the validator soft-launches parseable deprecations as warnings instead of rejecting builder-produced legacy configs.

Teach update diffing to treat legacy fieldConfigList[].indexType as unchanged when the stored JSON has an equivalent singleton indexTypes array, preserving idempotent updates after @JsonIgnore serialization.

Keep the pauseless / logical-table setStreamConfigs(null) regression fix from the amended commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xiangfu0 xiangfu0 force-pushed the validate-deprecated-table-config-create branch from a4a4b3f to e252bed Compare May 8, 2026 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

configuration Config changes (addition/deletion/change in behavior) deprecation Marks deprecated APIs, configs, or features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants