Copilot commented on code in PR #7640:
URL: https://github.com/apache/ignite-3/pull/7640#discussion_r2832165168
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableAddColumnCommand.java:
##########
@@ -125,6 +126,10 @@ private void validate() {
ensureTypeCanBeStored(column.name(), column.type());
ensureNonFunctionalDefault(column.name(),
column.defaultValueDefinition());
+
+ if (!column.nullable() && ((ConstantValue)
column.defaultValueDefinition()).value() == null) {
+ throw new CatalogValidationException("Non-nullable column '{}'
must have the default value.", column.name());
Review Comment:
This code performs an unsafe cast to ConstantValue without a null check or
type verification. While ensureNonFunctionalDefault on line 128 ensures the
default is not a FunctionCall, it allows null values to pass through. If
defaultValueDefinition is null, the cast will succeed but calling value() will
throw a NullPointerException instead of a meaningful validation error. Consider
adding an explicit null check or using instanceof before casting to provide
better error messages and ensure robustness.
```suggestion
Object defaultValueDefinition = column.defaultValueDefinition();
if (!column.nullable()) {
if (!(defaultValueDefinition instanceof ConstantValue)) {
throw new CatalogValidationException(
"Non-nullable column '{}' must have the default
value.",
column.name()
);
}
ConstantValue constantDefault = (ConstantValue)
defaultValueDefinition;
if (constantDefault.value() == null) {
throw new CatalogValidationException(
"Non-nullable column '{}' must have the default
value.",
column.name()
);
}
```
--
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]
