This is an automated email from the ASF dual-hosted git repository.
KKcorps pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 391f814e52c Add backward-incompatibility check for upsert
deleteRecordColumn (#18552)
391f814e52c is described below
commit 391f814e52ca18960f6470413e8a0a86cb484264
Author: Mayank Shrivastava <[email protected]>
AuthorDate: Thu May 21 07:40:43 2026 -0700
Add backward-incompatibility check for upsert deleteRecordColumn (#18552)
---
.../segment/local/utils/TableConfigUtils.java | 73 +++++++++++-----------
.../segment/local/utils/TableConfigUtilsTest.java | 55 ++++++++++++++++
2 files changed, 93 insertions(+), 35 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
index 31bea7e4a70..dc1d82c458f 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
@@ -1327,44 +1327,47 @@ public final class TableConfigUtils {
UpsertConfig existingUpsertConfig = existingConfig.getUpsertConfig();
UpsertConfig newUpsertConfig = newConfig.getUpsertConfig();
- if (existingUpsertConfig.getMode() != newUpsertConfig.getMode()) {
+ if (existingUpsertConfig.getMode() != newUpsertConfig.getMode()) {
+ violations.add(
+ String.format("upsertConfig.mode (%s -> %s)",
existingUpsertConfig.getMode(), newUpsertConfig.getMode()));
+ }
+ if (existingUpsertConfig.getHashFunction() !=
newUpsertConfig.getHashFunction()) {
+ violations.add(String.format("upsertConfig.hashFunction (%s -> %s)",
existingUpsertConfig.getHashFunction(),
+ newUpsertConfig.getHashFunction()));
+ }
+ if (!Objects.equals(existingUpsertConfig.getComparisonColumns(),
newUpsertConfig.getComparisonColumns())) {
+ violations.add(
+ String.format("upsertConfig.comparisonColumns (%s -> %s)",
existingUpsertConfig.getComparisonColumns(),
+ newUpsertConfig.getComparisonColumns()));
+ }
+ List<String> existingComparisonColumns =
existingUpsertConfig.getComparisonColumns();
+ if (existingComparisonColumns == null ||
existingComparisonColumns.isEmpty()) {
+ String existingTimeColumn =
+ existingConfig.getValidationConfig() != null ?
existingConfig.getValidationConfig().getTimeColumnName()
+ : null;
+ String newTimeColumn =
+ newConfig.getValidationConfig() != null ?
newConfig.getValidationConfig().getTimeColumnName() : null;
+ if (!Objects.equals(existingTimeColumn, newTimeColumn)) {
violations.add(
- String.format("upsertConfig.mode (%s -> %s)",
existingUpsertConfig.getMode(), newUpsertConfig.getMode()));
- }
- if (existingUpsertConfig.getHashFunction() !=
newUpsertConfig.getHashFunction()) {
- violations.add(String.format("upsertConfig.hashFunction (%s -> %s)",
existingUpsertConfig.getHashFunction(),
- newUpsertConfig.getHashFunction()));
- }
- if (!Objects.equals(existingUpsertConfig.getComparisonColumns(),
- newUpsertConfig.getComparisonColumns())) {
- violations.add(
- String.format("upsertConfig.comparisonColumns (%s -> %s)",
existingUpsertConfig.getComparisonColumns(),
- newUpsertConfig.getComparisonColumns()));
- }
- List<String> existingComparisonColumns =
existingUpsertConfig.getComparisonColumns();
- if (existingComparisonColumns == null ||
existingComparisonColumns.isEmpty()) {
- String existingTimeColumn =
- existingConfig.getValidationConfig() != null ?
existingConfig.getValidationConfig().getTimeColumnName()
- : null;
- String newTimeColumn =
- newConfig.getValidationConfig() != null ?
newConfig.getValidationConfig().getTimeColumnName() : null;
- if (!Objects.equals(existingTimeColumn, newTimeColumn)) {
- violations.add(
- String.format("timeColumnName (%s -> %s) - used as default
comparison column", existingTimeColumn,
- newTimeColumn));
- }
- }
- if (existingUpsertConfig.isDropOutOfOrderRecord() !=
newUpsertConfig.isDropOutOfOrderRecord()) {
- violations.add(
- String.format("upsertConfig.dropOutOfOrderRecord (%s -> %s)",
existingUpsertConfig.isDropOutOfOrderRecord(),
- newUpsertConfig.isDropOutOfOrderRecord()));
- }
- if (!Objects.equals(existingUpsertConfig.getOutOfOrderRecordColumn(),
- newUpsertConfig.getOutOfOrderRecordColumn())) {
- violations.add(String.format("upsertConfig.outOfOrderRecordColumn (%s
-> %s)",
- existingUpsertConfig.getOutOfOrderRecordColumn(),
newUpsertConfig.getOutOfOrderRecordColumn()));
+ String.format("timeColumnName (%s -> %s) - used as default
comparison column", existingTimeColumn,
+ newTimeColumn));
}
}
+ if (existingUpsertConfig.isDropOutOfOrderRecord() !=
newUpsertConfig.isDropOutOfOrderRecord()) {
+ violations.add(
+ String.format("upsertConfig.dropOutOfOrderRecord (%s -> %s)",
existingUpsertConfig.isDropOutOfOrderRecord(),
+ newUpsertConfig.isDropOutOfOrderRecord()));
+ }
+ if (!Objects.equals(existingUpsertConfig.getOutOfOrderRecordColumn(),
+ newUpsertConfig.getOutOfOrderRecordColumn())) {
+ violations.add(String.format("upsertConfig.outOfOrderRecordColumn (%s ->
%s)",
+ existingUpsertConfig.getOutOfOrderRecordColumn(),
newUpsertConfig.getOutOfOrderRecordColumn()));
+ }
+ if (!Objects.equals(existingUpsertConfig.getDeleteRecordColumn(),
newUpsertConfig.getDeleteRecordColumn())) {
+ violations.add(String.format("upsertConfig.deleteRecordColumn (%s ->
%s)",
+ existingUpsertConfig.getDeleteRecordColumn(),
newUpsertConfig.getDeleteRecordColumn()));
+ }
+ }
}
/**
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
index b6eac19916d..68555e127a9 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
@@ -4206,4 +4206,59 @@ public class TableConfigUtilsTest {
assertTrue(violations.isEmpty(),
"Expected no violations for partial-upsert strategy and
default-strategy changes, but got: " + violations);
}
+
+ @Test
+ public void
testValidateBackwardCompatibilityRejectsDeleteRecordColumnChange() {
+ UpsertConfig existingUpsertConfig = new
UpsertConfig(UpsertConfig.Mode.FULL);
+ existingUpsertConfig.setDeleteRecordColumn("deleted");
+ UpsertConfig newUpsertConfig = new UpsertConfig(UpsertConfig.Mode.FULL);
+ newUpsertConfig.setDeleteRecordColumn("is_deleted");
+
+ TableConfig existingConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
+
.setTimeColumnName(TIME_COLUMN).setUpsertConfig(existingUpsertConfig).build();
+ TableConfig newConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
+
.setTimeColumnName(TIME_COLUMN).setUpsertConfig(newUpsertConfig).build();
+
+ List<String> violations =
TableConfigUtils.validateBackwardCompatibility(newConfig, existingConfig);
+ assertEquals(violations.size(), 1);
+ assertTrue(violations.get(0).contains("deleteRecordColumn"));
+
+ // Adding deleteRecordColumn where none existed
+ existingUpsertConfig.setDeleteRecordColumn(null);
+ existingConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
+
.setTimeColumnName(TIME_COLUMN).setUpsertConfig(existingUpsertConfig).build();
+ violations = TableConfigUtils.validateBackwardCompatibility(newConfig,
existingConfig);
+ assertEquals(violations.size(), 1);
+ assertTrue(violations.get(0).contains("deleteRecordColumn"));
+
+ // Removing deleteRecordColumn
+ violations =
TableConfigUtils.validateBackwardCompatibility(existingConfig, newConfig);
+ assertEquals(violations.size(), 1);
+ assertTrue(violations.get(0).contains("deleteRecordColumn"));
+ }
+
+ @Test
+ public void testValidateBackwardCompatibilityAllowsIdenticalUpsertConfig() {
+ UpsertConfig upsertConfig = new UpsertConfig(UpsertConfig.Mode.FULL);
+ upsertConfig.setDeleteRecordColumn("deleted");
+
+ TableConfig existingConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
+ .setTimeColumnName(TIME_COLUMN).setUpsertConfig(upsertConfig).build();
+ TableConfig newConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
+ .setTimeColumnName(TIME_COLUMN).setUpsertConfig(upsertConfig).build();
+
+ List<String> violations =
TableConfigUtils.validateBackwardCompatibility(newConfig, existingConfig);
+ assertTrue(violations.isEmpty(), "Expected no violations for identical
config, but got: " + violations);
+ }
+
+ @Test
+ public void
testValidateBackwardCompatibilityNoViolationsForNonUpsertTables() {
+ TableConfig existingConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
+ .setTimeColumnName(TIME_COLUMN).build();
+ TableConfig newConfig = new
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
+ .setTimeColumnName(TIME_COLUMN).build();
+
+ List<String> violations =
TableConfigUtils.validateBackwardCompatibility(newConfig, existingConfig);
+ assertTrue(violations.isEmpty(), "Expected no violations for non-upsert
tables, but got: " + violations);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
