PDGGK opened a new pull request, #9292:
URL: https://github.com/apache/paimon/pull/9292
### Purpose
`ALTER TABLE ... ALTER COLUMN c TYPE ...` carries the column's existing
default value onto the new type without checking that the new type can read it:
```java
// SchemaManager:500-546, the UpdateColumnType branch
checkState(
DataTypeCasts.supportsCast(sourceRootType, targetRootType,
!disableExplicitTypeCasting)
&& CastExecutors.resolve(sourceRootType, targetRootType) !=
null,
...);
return new DataField(
field.id(),
field.name(),
getArrayMapTypeWithTargetTypeRoot(...),
field.description(),
field.defaultValue()); // <- not re-validated against the
new type
```
The type change itself is checked; the default value is not. Both other ways
of arriving at a `(type, default)` pair do check it:
* `CatalogUtils:172` validates every field on `createTable`;
* the `UpdateColumnDefaultValue` branch validates at `SchemaManager:601`.
So this path can commit a schema that neither of the other two would accept.
### What it does
A column declared `STRING DEFAULT 'abc'`, changed to `INT`:
```
alter accepted
field c type=INT default='abc'
validate(INT, 'abc') -> RuntimeException (createTable and SET DEFAULT
both reject this)
```
The table is then **unwritable**. `TableWriteImpl` builds its default-value
row in the constructor (`:88`), and `DefaultValueRow.create` converts every
default eagerly (`:237`):
```
table.newWrite("u")
-> java.lang.NumberFormatException: For input string: 'abc'. Invalid
character found.
```
Not the first write, not writes that omit the column — `newWrite` itself.
Every writer for that table fails to construct, and the DDL that caused it was
accepted without a warning.
Reads are unaffected: `DefaultValueRow.create` has exactly two callers,
`TableWriteImpl:88` and `FormatTableWrite:70`, both on the write side.
### What changes
Validate the carried-over default against the new type, the same call the
`UpdateColumnDefaultValue` branch already makes:
```java
DataType newFieldType =
getArrayMapTypeWithTargetTypeRoot(
field.type(), targetRootType, depth,
update.fieldNames().length);
validateDefaultValue(newFieldType, field.defaultValue());
return new DataField(
field.id(), field.name(), newFieldType, field.description(),
field.defaultValue());
```
`validateDefaultValue` returns immediately for a null default, so columns
without one are untouched, and a default the new type can read still goes
through — `STRING DEFAULT '123'` to `INT` is accepted and keeps the default.
The alter is **rejected** rather than the default being dropped or coerced.
Rejecting is what the two sibling paths do with the same pair, and silently
discarding a default the user set is worse than telling them to change it
first. A user who wants the change can update or clear the default in a
preceding statement.
Worth stating plainly: this refuses one thing that master allows — an alter
that leaves a default the new type cannot read. Every such alter produces a
table that cannot be written to, so no working case is lost, with the narrow
exception of a table that is never written to again. It also does not repair a
table already in that state; clearing or replacing the default still does.
### Test evidence
* `testUpdateColumnTypeRejectsADefaultValueTheNewTypeCannotRead` — `STRING
DEFAULT 'abc'` to `INT` throws with a `NumberFormatException` root cause, the
column is left as `STRING`, and `newWrite` still succeeds.
* `testUpdateColumnTypeKeepsADefaultValueTheNewTypeCanRead` — `STRING
DEFAULT '123'` to `INT` is accepted, the default survives as `'123'`, and
`newWrite` succeeds. This is the guard against fixing the bug by blocking every
alter on a defaulted column.
Mutation control, on a forced clean rebuild of `paimon-core` (`rm -rf
target/classes target/test-classes`): with the tests kept and `SchemaManager`
reverted, the rejection test fails — `Expecting code to raise a throwable` —
and the acceptance test stays green, which is what should happen since master
accepts that one too.
`SchemaManagerTest` — 43 tests, 0 failures. Wider run over the schema and
catalog tests in `paimon-core` — 735 tests, of which 734 pass; the one error is
`PostgresqlCatalogTest`, whose testcontainer will not start in my environment.
Verified against the unmodified base commit, where it fails identically.
### API and Format
No change to any option, on-disk format or public signature.
--
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]
