deepthi912 commented on code in PR #18552:
URL: https://github.com/apache/pinot/pull/18552#discussion_r3278516096
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -1316,55 +1317,68 @@ private static void
validateUpsertConfigUpdate(TableConfig newConfig, TableConfi
boolean existingUpsertEnabled = existingConfig.isUpsertEnabled();
boolean newUpsertEnabled = newConfig.isUpsertEnabled();
- // Check if upsert is being added or removed
if (existingUpsertEnabled != newUpsertEnabled) {
if (existingUpsertEnabled) {
- LOGGER.info("upsertConfig is removed from existing upsert table: {}",
newConfig.getTableName());
- } else {
- LOGGER.info("upsertConfig is added to existing non-upsert table: {}",
newConfig.getTableName());
- }
- } else if (existingUpsertEnabled) {
- UpsertConfig existingUpsertConfig = existingConfig.getUpsertConfig();
- UpsertConfig newUpsertConfig = newConfig.getUpsertConfig();
-
- 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())) {
+ String.format("Cannot remove upsertConfig from existing upsert
table: %s", newConfig.getTableName()));
+ } else {
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));
- }
+ String.format("Cannot add upsertConfig to existing non-upsert
table: %s", newConfig.getTableName()));
}
- if (existingUpsertConfig.isDropOutOfOrderRecord() !=
newUpsertConfig.isDropOutOfOrderRecord()) {
+ return;
+ }
+
+ if (!existingUpsertEnabled) {
+ return;
+ }
+
+ UpsertConfig existingUpsertConfig = existingConfig.getUpsertConfig();
+ UpsertConfig newUpsertConfig = newConfig.getUpsertConfig();
+
+ 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.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()));
+ }
+ if (existingUpsertConfig.getConsistencyMode() !=
newUpsertConfig.getConsistencyMode()) {
Review Comment:
This is still okay I think it only effects the way we see the data,
technically we shouldn't see any data corruption with these changes kicking in
imo.
--
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]
