rpuch commented on code in PR #3805:
URL: https://github.com/apache/ignite-3/pull/3805#discussion_r1611379973
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/CompatValidationResult.java:
##########
@@ -128,4 +142,16 @@ public int toSchemaVersion() {
return toSchemaVersion;
}
+
+ /**
+ * For failed validation returns details why the transition failed.
Review Comment:
It's not about a failed transition. It's about an operation
(read/write/commit) failing because the schema-at-operation-moment is
incompatible to the initial-schema.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/IncompatibleSchemaException.java:
##########
@@ -26,7 +26,58 @@
* because an incompatible schema change has happened.
*/
public class IncompatibleSchemaException extends TransactionException
implements ExpectedReplicationException {
+ private static final String SCHEMA_CHANGED_MESSAGE = "Table schema was
updated after the transaction was started "
+ + "[table=%s, startSchema=%d, operationSchema=%d]";
+
+ private static final String BACKWARDS_INCOMPATIBLE = "Schema is not
backward-compatible for table "
+ + "[table=%s, startSchema=%d, operationSchema=%d]";
+
+ private static final String TABLE_DROPPED_NAME_MESSAGE = "Table was
dropped [table=%s]";
+
+ private static final String TABLE_DROPPED_ID_MESSAGE = "Table was dropped
[tableId=%d]";
+
public IncompatibleSchemaException(String message) {
super(Transactions.TX_INCOMPATIBLE_SCHEMA_ERR, message);
}
+
+ public static IncompatibleSchemaException backwardsIncompatible(int
fromSchemaVersion, int toSchemaVersion, String failedTableName) {
Review Comment:
Javadoc is missing. Also, the method does not seem to be used at all
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -291,129 +333,169 @@ private enum ValidatorVerdict {
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ForwardCompatibilityValidator {
- ValidatorVerdict compatible(TableDefinitionDiff diff);
+ ValidatorResult compatible(TableDefinitionDiff diff);
}
private static class RenameTableValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Name of the table has been changed"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.nameDiffers() ? ValidatorVerdict.INCOMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.nameDiffers() ? INCOMPATIBLE :
ValidatorResult.DONT_CARE;
}
}
private static class AddColumnsValidator implements
ForwardCompatibilityValidator {
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
if (diff.addedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
+ boolean failed = false;
+
+ StringJoiner failedDetails = new StringJoiner(",", "Added nonnull
columns without default value: [", "]");
for (CatalogTableColumnDescriptor column : diff.addedColumns()) {
if (!column.nullable() && column.defaultValue() == null) {
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedDetails.add(column.name());
}
}
- return ValidatorVerdict.COMPATIBLE;
+ if (failed) {
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
failedDetails.toString());
+ }
+ return ValidatorResult.COMPATIBLE;
}
}
private static class DropColumnsValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Columns were dropped"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.removedColumns().isEmpty() ?
ValidatorVerdict.DONT_CARE : ValidatorVerdict.INCOMPATIBLE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.removedColumns().isEmpty() ? ValidatorResult.DONT_CARE
: INCOMPATIBLE;
}
}
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ColumnChangeCompatibilityValidator {
- ValidatorVerdict compatible(ColumnDefinitionDiff diff);
+ ValidatorResult compatible(ColumnDefinitionDiff diff);
}
private static class ChangeColumnsValidator implements
ForwardCompatibilityValidator {
- private final List<ColumnChangeCompatibilityValidator> validators =
List.of(
+ private static final List<ColumnChangeCompatibilityValidator>
validators = List.of(
// TODO: https://issues.apache.org/jira/browse/IGNITE-20948 -
add validator that says that column rename is compatible.
new ChangeNullabilityValidator(),
new ChangeDefaultValueValidator(),
new ChangeColumnTypeValidator()
);
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ ValidatorResult result = ValidatorResult.DONT_CARE;
+
if (diff.changedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return result;
Review Comment:
This is 'early return' pattern. Its benefit that it makes some trivial code
independent from non-trivial code that follows. Here, you introduce a
dependency on the `result` variable. I would suggest to get back to returning
`ValidatorVerdict.DONT_CARE` and declaring the variable after this `if` (if
declaring it at all).
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -291,129 +333,169 @@ private enum ValidatorVerdict {
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ForwardCompatibilityValidator {
- ValidatorVerdict compatible(TableDefinitionDiff diff);
+ ValidatorResult compatible(TableDefinitionDiff diff);
}
private static class RenameTableValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Name of the table has been changed"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.nameDiffers() ? ValidatorVerdict.INCOMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.nameDiffers() ? INCOMPATIBLE :
ValidatorResult.DONT_CARE;
}
}
private static class AddColumnsValidator implements
ForwardCompatibilityValidator {
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
if (diff.addedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
+ boolean failed = false;
+
+ StringJoiner failedDetails = new StringJoiner(",", "Added nonnull
columns without default value: [", "]");
for (CatalogTableColumnDescriptor column : diff.addedColumns()) {
if (!column.nullable() && column.defaultValue() == null) {
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedDetails.add(column.name());
}
}
- return ValidatorVerdict.COMPATIBLE;
+ if (failed) {
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
failedDetails.toString());
+ }
+ return ValidatorResult.COMPATIBLE;
}
}
private static class DropColumnsValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Columns were dropped"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.removedColumns().isEmpty() ?
ValidatorVerdict.DONT_CARE : ValidatorVerdict.INCOMPATIBLE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.removedColumns().isEmpty() ? ValidatorResult.DONT_CARE
: INCOMPATIBLE;
}
}
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ColumnChangeCompatibilityValidator {
- ValidatorVerdict compatible(ColumnDefinitionDiff diff);
+ ValidatorResult compatible(ColumnDefinitionDiff diff);
}
private static class ChangeColumnsValidator implements
ForwardCompatibilityValidator {
- private final List<ColumnChangeCompatibilityValidator> validators =
List.of(
+ private static final List<ColumnChangeCompatibilityValidator>
validators = List.of(
// TODO: https://issues.apache.org/jira/browse/IGNITE-20948 -
add validator that says that column rename is compatible.
new ChangeNullabilityValidator(),
new ChangeDefaultValueValidator(),
new ChangeColumnTypeValidator()
);
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ ValidatorResult result = ValidatorResult.DONT_CARE;
+
if (diff.changedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return result;
}
- boolean accepted = false;
+ boolean failed = false;
+ StringJoiner failedString = new StringJoiner("; ", "Columns change
validation failed: [", "]");
Review Comment:
```suggestion
StringJoiner failedString = new StringJoiner("; ", "Columns
definition changed incompatibly: [", "]");
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -144,38 +147,56 @@ private CompatValidationResult
validateForwardSchemaCompatibility(
for (int i = 0; i < tableSchemas.size() - 1; i++) {
FullTableSchema oldSchema = tableSchemas.get(i);
FullTableSchema newSchema = tableSchemas.get(i + 1);
- if (!isForwardCompatible(oldSchema, newSchema)) {
- return CompatValidationResult.incompatibleChange(tableId,
oldSchema.schemaVersion(), newSchema.schemaVersion());
+
+ List<String> failedValidationDetails =
getFailedValidationDetails(oldSchema, newSchema);
+
+ if (!failedValidationDetails.isEmpty()) {
+ return CompatValidationResult.incompatibleChange(
+ oldSchema.tableName(),
+ oldSchema.schemaVersion(),
+ newSchema.schemaVersion(),
+ String.join("; ", failedValidationDetails)
+ );
}
}
return CompatValidationResult.success();
}
- private boolean isForwardCompatible(FullTableSchema prevSchema,
FullTableSchema nextSchema) {
+ private List<String> getFailedValidationDetails(FullTableSchema
prevSchema, FullTableSchema nextSchema) {
TableDefinitionDiff diff = diffCache.computeIfAbsent(
new TableDefinitionDiffKey(prevSchema.tableId(),
prevSchema.schemaVersion(), nextSchema.schemaVersion()),
key -> nextSchema.diffFrom(prevSchema)
);
- boolean accepted = false;
+ boolean failed = false;
+ ValidatorResult result = ValidatorResult.DONT_CARE;
+ List<String> failedMessages = new ArrayList<>();
for (ForwardCompatibilityValidator validator :
FORWARD_COMPATIBILITY_VALIDATORS) {
- switch (validator.compatible(diff)) {
+ ValidatorResult validatorResult = validator.compatible(diff);
+ switch (validatorResult.verdict) {
case COMPATIBLE:
- accepted = true;
+ result = ValidatorResult.COMPATIBLE;
break;
case INCOMPATIBLE:
- return false;
+ failed = true;
+ failedMessages.add(validatorResult.details);
+ break;
default:
break;
}
}
- assert accepted : "Table schema changed from " +
prevSchema.schemaVersion() + " and " + nextSchema.schemaVersion()
+ if (failed) {
+ return failedMessages;
+ }
+
+ assert result == ValidatorResult.COMPATIBLE : "Table schema changed
from " + prevSchema.schemaVersion()
+ + " and " + nextSchema.schemaVersion()
Review Comment:
```suggestion
+ " to " + nextSchema.schemaVersion()
```
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidatorTest.java:
##########
@@ -300,11 +309,22 @@ private static void addInconvertible(ColumnType type1,
ColumnType type2, List<Co
@Test
void combinationOfForwardCompatibleChangesIsCompatible() {
- assertForwardCompatibleChangeAllowsCommitting(() -> List.of(
- tableSchema(1, List.of(column(INT32, false))),
- // Type is widened, NOT NULL dropped.
- tableSchema(2, List.of(column(INT64, true)))
- ));
+ assertForwardCompatibleChangeAllowsCommitting(new SchemaChangeSource()
{
+ @Override
+ public List<FullTableSchema> schemaVersions() {
+ return List.of(
+ tableSchema(1, List.of(column(INT32, false))),
+ // Type is widened, NOT NULL dropped.
+ tableSchema(2, List.of(column(INT64, true)))
+ );
+ }
+
+ @Override
+ // Not valid for compatible changes.
+ public List<String> expectedDetails() {
Review Comment:
How about adding `CompatibleSchemaChangeSource` defining this method in this
way, so we don't need to repeat it everywhere?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -144,38 +147,56 @@ private CompatValidationResult
validateForwardSchemaCompatibility(
for (int i = 0; i < tableSchemas.size() - 1; i++) {
FullTableSchema oldSchema = tableSchemas.get(i);
FullTableSchema newSchema = tableSchemas.get(i + 1);
- if (!isForwardCompatible(oldSchema, newSchema)) {
- return CompatValidationResult.incompatibleChange(tableId,
oldSchema.schemaVersion(), newSchema.schemaVersion());
+
+ List<String> failedValidationDetails =
getFailedValidationDetails(oldSchema, newSchema);
+
+ if (!failedValidationDetails.isEmpty()) {
+ return CompatValidationResult.incompatibleChange(
+ oldSchema.tableName(),
+ oldSchema.schemaVersion(),
+ newSchema.schemaVersion(),
+ String.join("; ", failedValidationDetails)
+ );
}
}
return CompatValidationResult.success();
}
- private boolean isForwardCompatible(FullTableSchema prevSchema,
FullTableSchema nextSchema) {
+ private List<String> getFailedValidationDetails(FullTableSchema
prevSchema, FullTableSchema nextSchema) {
TableDefinitionDiff diff = diffCache.computeIfAbsent(
new TableDefinitionDiffKey(prevSchema.tableId(),
prevSchema.schemaVersion(), nextSchema.schemaVersion()),
key -> nextSchema.diffFrom(prevSchema)
);
- boolean accepted = false;
+ boolean failed = false;
+ ValidatorResult result = ValidatorResult.DONT_CARE;
Review Comment:
Switching from storing `accepted` to storing `result` makes it a bit more
confusing, it seems. When it's just a boolean you realize that the only
information it has is whether it was accepted or not. When having an enum, this
already means 'more information', so it's more difficult to process it when
reading the code to realize why it's here.
Could you change it back? Maybe renaming the variable from `accepted` to
`acceptedAtLeastOnce`
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -1622,15 +1622,16 @@ private static void
throwIfSchemaValidationOnCommitFailed(CompatValidationResult
if (validationResult.isTableDropped()) {
// TODO: IGNITE-20966 - improve error message.
throw new MismatchingTransactionOutcomeException(
- format("Commit failed because a table was already
dropped [tableId={}]", validationResult.failedTableId()),
+ format("Commit failed because a table was already
dropped [table={}]", validationResult.failedTableName()),
txResult
);
} else {
// TODO: IGNITE-20966 - improve error message.
throw new MismatchingTransactionOutcomeException(
"Commit failed because schema "
Review Comment:
It is possible to reformat the error message to conform to AI3 guidelines?
That is, that the error message looks like 'Bla bla [k1=v1, k2=v2, ..]'.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -144,38 +147,56 @@ private CompatValidationResult
validateForwardSchemaCompatibility(
for (int i = 0; i < tableSchemas.size() - 1; i++) {
FullTableSchema oldSchema = tableSchemas.get(i);
FullTableSchema newSchema = tableSchemas.get(i + 1);
- if (!isForwardCompatible(oldSchema, newSchema)) {
- return CompatValidationResult.incompatibleChange(tableId,
oldSchema.schemaVersion(), newSchema.schemaVersion());
+
+ List<String> failedValidationDetails =
getFailedValidationDetails(oldSchema, newSchema);
+
+ if (!failedValidationDetails.isEmpty()) {
+ return CompatValidationResult.incompatibleChange(
+ oldSchema.tableName(),
+ oldSchema.schemaVersion(),
+ newSchema.schemaVersion(),
+ String.join("; ", failedValidationDetails)
+ );
}
}
return CompatValidationResult.success();
}
- private boolean isForwardCompatible(FullTableSchema prevSchema,
FullTableSchema nextSchema) {
+ private List<String> getFailedValidationDetails(FullTableSchema
prevSchema, FullTableSchema nextSchema) {
TableDefinitionDiff diff = diffCache.computeIfAbsent(
new TableDefinitionDiffKey(prevSchema.tableId(),
prevSchema.schemaVersion(), nextSchema.schemaVersion()),
key -> nextSchema.diffFrom(prevSchema)
);
- boolean accepted = false;
+ boolean failed = false;
+ ValidatorResult result = ValidatorResult.DONT_CARE;
+ List<String> failedMessages = new ArrayList<>();
for (ForwardCompatibilityValidator validator :
FORWARD_COMPATIBILITY_VALIDATORS) {
- switch (validator.compatible(diff)) {
+ ValidatorResult validatorResult = validator.compatible(diff);
+ switch (validatorResult.verdict) {
case COMPATIBLE:
- accepted = true;
+ result = ValidatorResult.COMPATIBLE;
break;
case INCOMPATIBLE:
- return false;
+ failed = true;
+ failedMessages.add(validatorResult.details);
Review Comment:
Why do we want to collect every failure? This is not a UI validation where
we would want to help the user fix everything. Commit validation failures are
transient (that is, if a transaction is repeated, it will already start on the
new schema and will not be failed due to the same schema inconsistency). So it
seems that we need to just return the first failure that we encounter.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -291,129 +333,169 @@ private enum ValidatorVerdict {
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ForwardCompatibilityValidator {
- ValidatorVerdict compatible(TableDefinitionDiff diff);
+ ValidatorResult compatible(TableDefinitionDiff diff);
}
private static class RenameTableValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Name of the table has been changed"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.nameDiffers() ? ValidatorVerdict.INCOMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.nameDiffers() ? INCOMPATIBLE :
ValidatorResult.DONT_CARE;
}
}
private static class AddColumnsValidator implements
ForwardCompatibilityValidator {
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
if (diff.addedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
+ boolean failed = false;
+
+ StringJoiner failedDetails = new StringJoiner(",", "Added nonnull
columns without default value: [", "]");
for (CatalogTableColumnDescriptor column : diff.addedColumns()) {
if (!column.nullable() && column.defaultValue() == null) {
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedDetails.add(column.name());
}
}
- return ValidatorVerdict.COMPATIBLE;
+ if (failed) {
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
failedDetails.toString());
+ }
+ return ValidatorResult.COMPATIBLE;
}
}
private static class DropColumnsValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Columns were dropped"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.removedColumns().isEmpty() ?
ValidatorVerdict.DONT_CARE : ValidatorVerdict.INCOMPATIBLE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.removedColumns().isEmpty() ? ValidatorResult.DONT_CARE
: INCOMPATIBLE;
}
}
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ColumnChangeCompatibilityValidator {
- ValidatorVerdict compatible(ColumnDefinitionDiff diff);
+ ValidatorResult compatible(ColumnDefinitionDiff diff);
}
private static class ChangeColumnsValidator implements
ForwardCompatibilityValidator {
- private final List<ColumnChangeCompatibilityValidator> validators =
List.of(
+ private static final List<ColumnChangeCompatibilityValidator>
validators = List.of(
// TODO: https://issues.apache.org/jira/browse/IGNITE-20948 -
add validator that says that column rename is compatible.
new ChangeNullabilityValidator(),
new ChangeDefaultValueValidator(),
new ChangeColumnTypeValidator()
);
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ ValidatorResult result = ValidatorResult.DONT_CARE;
+
if (diff.changedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return result;
}
- boolean accepted = false;
+ boolean failed = false;
+ StringJoiner failedString = new StringJoiner("; ", "Columns change
validation failed: [", "]");
for (ColumnDefinitionDiff columnDiff : diff.changedColumns()) {
- switch (compatible(columnDiff)) {
+ ValidatorResult validatorResult = compatible(columnDiff);
+ switch (validatorResult.verdict()) {
case COMPATIBLE:
- accepted = true;
+ result = ValidatorResult.COMPATIBLE;
break;
case INCOMPATIBLE:
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedString.add(validatorResult.details());
Review Comment:
Same thing about returning first failure immediately
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/IncompatibleSchemaException.java:
##########
@@ -26,7 +26,58 @@
* because an incompatible schema change has happened.
*/
public class IncompatibleSchemaException extends TransactionException
implements ExpectedReplicationException {
+ private static final String SCHEMA_CHANGED_MESSAGE = "Table schema was
updated after the transaction was started "
+ + "[table=%s, startSchema=%d, operationSchema=%d]";
+
+ private static final String BACKWARDS_INCOMPATIBLE = "Schema is not
backward-compatible for table "
+ + "[table=%s, startSchema=%d, operationSchema=%d]";
+
+ private static final String TABLE_DROPPED_NAME_MESSAGE = "Table was
dropped [table=%s]";
+
+ private static final String TABLE_DROPPED_ID_MESSAGE = "Table was dropped
[tableId=%d]";
+
public IncompatibleSchemaException(String message) {
super(Transactions.TX_INCOMPATIBLE_SCHEMA_ERR, message);
}
+
+ public static IncompatibleSchemaException backwardsIncompatible(int
fromSchemaVersion, int toSchemaVersion, String failedTableName) {
+ return new
IncompatibleSchemaException(String.format(BACKWARDS_INCOMPATIBLE,
failedTableName, fromSchemaVersion, toSchemaVersion));
+ }
+
+ /**
+ * Returns new IncompatibleSchemaException for a case when schema was
updated after the beginning of the transaction.
+ *
+ * @param tableName Name of the table.
+ * @param startSchemaVersion Schema version at the beginning of the
transaction.
+ * @param operationSchemaVersion Schema version at the moment of the
operation.
+ * @return Exception with formatted message.
+ */
+ public static IncompatibleSchemaException schemaChanged(String tableName,
int startSchemaVersion, int operationSchemaVersion) {
+ return new IncompatibleSchemaException(String.format(
+ SCHEMA_CHANGED_MESSAGE,
+ tableName, startSchemaVersion, operationSchemaVersion
+ ));
+ }
+
+ /**
+ * Returns new IncompatibleSchemaException for a case when was dropped at
the moment of operation.
Review Comment:
```suggestion
* Returns new IncompatibleSchemaException for a case when the table was
dropped at the moment of operation.
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -291,129 +333,169 @@ private enum ValidatorVerdict {
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ForwardCompatibilityValidator {
- ValidatorVerdict compatible(TableDefinitionDiff diff);
+ ValidatorResult compatible(TableDefinitionDiff diff);
}
private static class RenameTableValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Name of the table has been changed"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.nameDiffers() ? ValidatorVerdict.INCOMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.nameDiffers() ? INCOMPATIBLE :
ValidatorResult.DONT_CARE;
}
}
private static class AddColumnsValidator implements
ForwardCompatibilityValidator {
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
if (diff.addedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
+ boolean failed = false;
+
+ StringJoiner failedDetails = new StringJoiner(",", "Added nonnull
columns without default value: [", "]");
for (CatalogTableColumnDescriptor column : diff.addedColumns()) {
if (!column.nullable() && column.defaultValue() == null) {
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedDetails.add(column.name());
Review Comment:
Same thing: first failure is enough, we don't need to collect every failure
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -274,6 +295,27 @@ void failIfRequestSchemaDiffersFromTxTs(HybridTimestamp
txTs, int requestSchemaV
}
}
+ private static class ValidatorResult {
Review Comment:
```suggestion
private static class ValidationResult {
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/IncompatibleSchemaException.java:
##########
@@ -26,7 +26,58 @@
* because an incompatible schema change has happened.
*/
public class IncompatibleSchemaException extends TransactionException
implements ExpectedReplicationException {
+ private static final String SCHEMA_CHANGED_MESSAGE = "Table schema was
updated after the transaction was started "
+ + "[table=%s, startSchema=%d, operationSchema=%d]";
+
+ private static final String BACKWARDS_INCOMPATIBLE = "Schema is not
backward-compatible for table "
+ + "[table=%s, startSchema=%d, operationSchema=%d]";
+
+ private static final String TABLE_DROPPED_NAME_MESSAGE = "Table was
dropped [table=%s]";
+
+ private static final String TABLE_DROPPED_ID_MESSAGE = "Table was dropped
[tableId=%d]";
+
public IncompatibleSchemaException(String message) {
super(Transactions.TX_INCOMPATIBLE_SCHEMA_ERR, message);
}
+
+ public static IncompatibleSchemaException backwardsIncompatible(int
fromSchemaVersion, int toSchemaVersion, String failedTableName) {
+ return new
IncompatibleSchemaException(String.format(BACKWARDS_INCOMPATIBLE,
failedTableName, fromSchemaVersion, toSchemaVersion));
+ }
+
+ /**
+ * Returns new IncompatibleSchemaException for a case when schema was
updated after the beginning of the transaction.
+ *
+ * @param tableName Name of the table.
+ * @param startSchemaVersion Schema version at the beginning of the
transaction.
+ * @param operationSchemaVersion Schema version at the moment of the
operation.
+ * @return Exception with formatted message.
+ */
+ public static IncompatibleSchemaException schemaChanged(String tableName,
int startSchemaVersion, int operationSchemaVersion) {
+ return new IncompatibleSchemaException(String.format(
+ SCHEMA_CHANGED_MESSAGE,
+ tableName, startSchemaVersion, operationSchemaVersion
+ ));
+ }
+
+ /**
+ * Returns new IncompatibleSchemaException for a case when was dropped at
the moment of operation.
+ *
+ * @param tableName Name of the table.
+ * @return Exception with formatted message.
+ */
+ public static IncompatibleSchemaException tableDropped(String tableName) {
+ return new
IncompatibleSchemaException(String.format(TABLE_DROPPED_NAME_MESSAGE,
tableName));
+ }
+
+ /**
+ * Returns new IncompatibleSchemaException for a case when table was
dropped at the moment of operation.
+ *
+ * @param tableId ID of the table.
+ * @return Exception with formatted message.
+ */
+ // TODO https://issues.apache.org/jira/browse/IGNITE-22309 use tableName
instead
+ @Deprecated
Review Comment:
I'm not sure it's even possible to consistently resolve table name, so the
deprecation is probably too much.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -1622,15 +1622,16 @@ private static void
throwIfSchemaValidationOnCommitFailed(CompatValidationResult
if (validationResult.isTableDropped()) {
// TODO: IGNITE-20966 - improve error message.
Review Comment:
These TODOs must be removed. Please find and remove them all.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -291,129 +333,169 @@ private enum ValidatorVerdict {
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ForwardCompatibilityValidator {
- ValidatorVerdict compatible(TableDefinitionDiff diff);
+ ValidatorResult compatible(TableDefinitionDiff diff);
}
private static class RenameTableValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Name of the table has been changed"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.nameDiffers() ? ValidatorVerdict.INCOMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.nameDiffers() ? INCOMPATIBLE :
ValidatorResult.DONT_CARE;
}
}
private static class AddColumnsValidator implements
ForwardCompatibilityValidator {
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
if (diff.addedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
+ boolean failed = false;
+
+ StringJoiner failedDetails = new StringJoiner(",", "Added nonnull
columns without default value: [", "]");
for (CatalogTableColumnDescriptor column : diff.addedColumns()) {
if (!column.nullable() && column.defaultValue() == null) {
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedDetails.add(column.name());
}
}
- return ValidatorVerdict.COMPATIBLE;
+ if (failed) {
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
failedDetails.toString());
+ }
+ return ValidatorResult.COMPATIBLE;
}
}
private static class DropColumnsValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Columns were dropped"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.removedColumns().isEmpty() ?
ValidatorVerdict.DONT_CARE : ValidatorVerdict.INCOMPATIBLE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.removedColumns().isEmpty() ? ValidatorResult.DONT_CARE
: INCOMPATIBLE;
}
}
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ColumnChangeCompatibilityValidator {
- ValidatorVerdict compatible(ColumnDefinitionDiff diff);
+ ValidatorResult compatible(ColumnDefinitionDiff diff);
}
private static class ChangeColumnsValidator implements
ForwardCompatibilityValidator {
- private final List<ColumnChangeCompatibilityValidator> validators =
List.of(
+ private static final List<ColumnChangeCompatibilityValidator>
validators = List.of(
// TODO: https://issues.apache.org/jira/browse/IGNITE-20948 -
add validator that says that column rename is compatible.
new ChangeNullabilityValidator(),
new ChangeDefaultValueValidator(),
new ChangeColumnTypeValidator()
);
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ ValidatorResult result = ValidatorResult.DONT_CARE;
+
if (diff.changedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return result;
}
- boolean accepted = false;
+ boolean failed = false;
+ StringJoiner failedString = new StringJoiner("; ", "Columns change
validation failed: [", "]");
for (ColumnDefinitionDiff columnDiff : diff.changedColumns()) {
- switch (compatible(columnDiff)) {
+ ValidatorResult validatorResult = compatible(columnDiff);
+ switch (validatorResult.verdict()) {
case COMPATIBLE:
- accepted = true;
+ result = ValidatorResult.COMPATIBLE;
break;
case INCOMPATIBLE:
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedString.add(validatorResult.details());
+ break;
default:
break;
}
}
- assert accepted : "Table schema changed from " +
diff.oldSchemaVersion() + " and " + diff.newSchemaVersion()
- + ", but no column change validator voted for any change.
Some schema validator is missing.";
+ if (failed) {
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
failedString.toString());
+ }
+
+ assert result == ValidatorResult.COMPATIBLE : "Table schema
changed from " + diff.oldSchemaVersion() + " and "
+ + diff.newSchemaVersion() + ", but no column change
validator voted for any change. Some schema validator is missing.";
- return ValidatorVerdict.COMPATIBLE;
+ return result;
}
- private ValidatorVerdict compatible(ColumnDefinitionDiff columnDiff) {
- boolean accepted = false;
+ private static ValidatorResult compatible(ColumnDefinitionDiff
columnDiff) {
+ StringJoiner columnVerdict = new StringJoiner(", ",
columnDiff.oldName() + ": ", "");
+ boolean failed = false;
+
+ ValidatorResult result = ValidatorResult.DONT_CARE;
for (ColumnChangeCompatibilityValidator validator : validators) {
- switch (validator.compatible(columnDiff)) {
+ ValidatorResult validatorResult =
validator.compatible(columnDiff);
+
+ switch (validatorResult.verdict()) {
case COMPATIBLE:
- accepted = true;
+ result = ValidatorResult.COMPATIBLE;
break;
case INCOMPATIBLE:
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ columnVerdict.add(validatorResult.details());
+ break;
default:
break;
}
}
- return accepted ? ValidatorVerdict.COMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ return failed ? new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
columnVerdict.toString()) : result;
}
}
private static class ChangeDefaultValueValidator implements
ColumnChangeCompatibilityValidator {
@Override
- public ValidatorVerdict compatible(ColumnDefinitionDiff diff) {
- return diff.defaultChanged() ? ValidatorVerdict.INCOMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ public ValidatorResult compatible(ColumnDefinitionDiff diff) {
+ return diff.defaultChanged()
+ ? new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
"Default value changed")
+ : ValidatorResult.DONT_CARE;
}
}
private static class ChangeNullabilityValidator implements
ColumnChangeCompatibilityValidator {
@Override
- public ValidatorVerdict compatible(ColumnDefinitionDiff diff) {
+ public ValidatorResult compatible(ColumnDefinitionDiff diff) {
if (diff.notNullAdded()) {
- return ValidatorVerdict.INCOMPATIBLE;
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE, "Not
null added");
}
+
if (diff.notNullDropped()) {
- return ValidatorVerdict.COMPATIBLE;
+ return ValidatorResult.COMPATIBLE;
}
assert !diff.nullabilityChanged() : diff;
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
}
private static class ChangeColumnTypeValidator implements
ColumnChangeCompatibilityValidator {
@Override
- public ValidatorVerdict compatible(ColumnDefinitionDiff diff) {
+ public ValidatorResult compatible(ColumnDefinitionDiff diff) {
if (!diff.typeChanged()) {
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
- return diff.typeChangeIsSupported() ? ValidatorVerdict.COMPATIBLE
: ValidatorVerdict.INCOMPATIBLE;
+ return diff.typeChangeIsSupported()
+ ? ValidatorResult.COMPATIBLE
+ : new ValidatorResult(ValidatorVerdict.INCOMPATIBLE, "Type
changed");
Review Comment:
```suggestion
: new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
"Type changed incompatibly");
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -291,129 +333,169 @@ private enum ValidatorVerdict {
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ForwardCompatibilityValidator {
- ValidatorVerdict compatible(TableDefinitionDiff diff);
+ ValidatorResult compatible(TableDefinitionDiff diff);
}
private static class RenameTableValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Name of the table has been changed"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.nameDiffers() ? ValidatorVerdict.INCOMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.nameDiffers() ? INCOMPATIBLE :
ValidatorResult.DONT_CARE;
}
}
private static class AddColumnsValidator implements
ForwardCompatibilityValidator {
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
if (diff.addedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
+ boolean failed = false;
+
+ StringJoiner failedDetails = new StringJoiner(",", "Added nonnull
columns without default value: [", "]");
for (CatalogTableColumnDescriptor column : diff.addedColumns()) {
if (!column.nullable() && column.defaultValue() == null) {
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedDetails.add(column.name());
}
}
- return ValidatorVerdict.COMPATIBLE;
+ if (failed) {
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
failedDetails.toString());
+ }
+ return ValidatorResult.COMPATIBLE;
}
}
private static class DropColumnsValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Columns were dropped"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.removedColumns().isEmpty() ?
ValidatorVerdict.DONT_CARE : ValidatorVerdict.INCOMPATIBLE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.removedColumns().isEmpty() ? ValidatorResult.DONT_CARE
: INCOMPATIBLE;
}
}
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ColumnChangeCompatibilityValidator {
- ValidatorVerdict compatible(ColumnDefinitionDiff diff);
+ ValidatorResult compatible(ColumnDefinitionDiff diff);
}
private static class ChangeColumnsValidator implements
ForwardCompatibilityValidator {
- private final List<ColumnChangeCompatibilityValidator> validators =
List.of(
+ private static final List<ColumnChangeCompatibilityValidator>
validators = List.of(
// TODO: https://issues.apache.org/jira/browse/IGNITE-20948 -
add validator that says that column rename is compatible.
new ChangeNullabilityValidator(),
new ChangeDefaultValueValidator(),
new ChangeColumnTypeValidator()
);
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ ValidatorResult result = ValidatorResult.DONT_CARE;
+
if (diff.changedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return result;
}
- boolean accepted = false;
Review Comment:
Same thing about `accepted` and `result`: we don't actually need the latter
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/SchemaCompatibilityValidator.java:
##########
@@ -291,129 +333,169 @@ private enum ValidatorVerdict {
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ForwardCompatibilityValidator {
- ValidatorVerdict compatible(TableDefinitionDiff diff);
+ ValidatorResult compatible(TableDefinitionDiff diff);
}
private static class RenameTableValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Name of the table has been changed"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.nameDiffers() ? ValidatorVerdict.INCOMPATIBLE :
ValidatorVerdict.DONT_CARE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.nameDiffers() ? INCOMPATIBLE :
ValidatorResult.DONT_CARE;
}
}
private static class AddColumnsValidator implements
ForwardCompatibilityValidator {
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
if (diff.addedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return ValidatorResult.DONT_CARE;
}
+ boolean failed = false;
+
+ StringJoiner failedDetails = new StringJoiner(",", "Added nonnull
columns without default value: [", "]");
for (CatalogTableColumnDescriptor column : diff.addedColumns()) {
if (!column.nullable() && column.defaultValue() == null) {
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedDetails.add(column.name());
}
}
- return ValidatorVerdict.COMPATIBLE;
+ if (failed) {
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
failedDetails.toString());
+ }
+ return ValidatorResult.COMPATIBLE;
}
}
private static class DropColumnsValidator implements
ForwardCompatibilityValidator {
+ private static final ValidatorResult INCOMPATIBLE = new
ValidatorResult(
+ ValidatorVerdict.INCOMPATIBLE,
+ "Columns were dropped"
+ );
+
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
- return diff.removedColumns().isEmpty() ?
ValidatorVerdict.DONT_CARE : ValidatorVerdict.INCOMPATIBLE;
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ return diff.removedColumns().isEmpty() ? ValidatorResult.DONT_CARE
: INCOMPATIBLE;
}
}
@SuppressWarnings("InterfaceMayBeAnnotatedFunctional")
private interface ColumnChangeCompatibilityValidator {
- ValidatorVerdict compatible(ColumnDefinitionDiff diff);
+ ValidatorResult compatible(ColumnDefinitionDiff diff);
}
private static class ChangeColumnsValidator implements
ForwardCompatibilityValidator {
- private final List<ColumnChangeCompatibilityValidator> validators =
List.of(
+ private static final List<ColumnChangeCompatibilityValidator>
validators = List.of(
// TODO: https://issues.apache.org/jira/browse/IGNITE-20948 -
add validator that says that column rename is compatible.
new ChangeNullabilityValidator(),
new ChangeDefaultValueValidator(),
new ChangeColumnTypeValidator()
);
@Override
- public ValidatorVerdict compatible(TableDefinitionDiff diff) {
+ public ValidatorResult compatible(TableDefinitionDiff diff) {
+ ValidatorResult result = ValidatorResult.DONT_CARE;
+
if (diff.changedColumns().isEmpty()) {
- return ValidatorVerdict.DONT_CARE;
+ return result;
}
- boolean accepted = false;
+ boolean failed = false;
+ StringJoiner failedString = new StringJoiner("; ", "Columns change
validation failed: [", "]");
for (ColumnDefinitionDiff columnDiff : diff.changedColumns()) {
- switch (compatible(columnDiff)) {
+ ValidatorResult validatorResult = compatible(columnDiff);
+ switch (validatorResult.verdict()) {
case COMPATIBLE:
- accepted = true;
+ result = ValidatorResult.COMPATIBLE;
break;
case INCOMPATIBLE:
- return ValidatorVerdict.INCOMPATIBLE;
+ failed = true;
+ failedString.add(validatorResult.details());
+ break;
default:
break;
}
}
- assert accepted : "Table schema changed from " +
diff.oldSchemaVersion() + " and " + diff.newSchemaVersion()
- + ", but no column change validator voted for any change.
Some schema validator is missing.";
+ if (failed) {
+ return new ValidatorResult(ValidatorVerdict.INCOMPATIBLE,
failedString.toString());
+ }
+
+ assert result == ValidatorResult.COMPATIBLE : "Table schema
changed from " + diff.oldSchemaVersion() + " and "
+ + diff.newSchemaVersion() + ", but no column change
validator voted for any change. Some schema validator is missing.";
- return ValidatorVerdict.COMPATIBLE;
+ return result;
}
- private ValidatorVerdict compatible(ColumnDefinitionDiff columnDiff) {
- boolean accepted = false;
Review Comment:
Same thing about multiple failures and `accepted`
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
