morningman commented on issue #65383:
URL: https://github.com/apache/doris/issues/65383#issuecomment-5520570673
This is no longer reproducible on current master. It was fixed as a side
effect of #65810 (commit `f745ddf9e22`, 2026-08-07), about a month after this
issue was filed.
### Root cause at the time
`AlterOperations.checkBinlogConfigChange()` only recognized `binlog.enable`,
`binlog.ttl_seconds`, `binlog.max_bytes` and `binlog.max_history_nums`.
`binlog.format` was not in that list, so `ALTER TABLE ... SET ("binlog.format"
= ...)` fell through the binlog branch of
`Alter.processAlterOlapTableInternal()` and was dispatched to
`hasSchemaChangeOp()` → `SchemaChangeHandler.process()`, where the row-binlog
light-schema-change guard rejected it with the generic message. The dedicated
check in `BinlogConfig.mergeFromProperties()` was never reached, which is why
"not support change binlog format" never showed up.
### The fix
`f745ddf9e22` added the two missing keys:
```java
// fe/fe-core/src/main/java/org/apache/doris/alter/AlterOperations.java
+ ||
clause.getProperties().containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_FORMAT)
+ ||
clause.getProperties().containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_NEED_HISTORICAL_VALUE));
```
On current master the statement routes to
`SchemaChangeHandler.updateBinlogConfig()` →
`BinlogConfig.mergeFromProperties(properties, force = false)` and fails with:
```
errCode = 2, detailMessage = not support change binlog format from ROW to
STATEMENT_AND_SNAPSHOT
```
which is what the case expects. The same commit also fixed
`binlog.need_historical_value`, which had exactly the same problem.
I verified this on current master (`71a63249bbd`) with a local FE unit test
built on `TestWithFeService`, running the exact statements from the report
against a `binlog.format = ROW` MOW table:
| statement | error message on current master |
| --- | --- |
| `SET ("binlog.format" = "STATEMENT_AND_SNAPSHOT")` | `not support change
binlog format from ROW to STATEMENT_AND_SNAPSHOT` |
| `SET ("binlog.need_historical_value" = "false")` | `not support change
binlog.need_historical_value from true to false` |
| `SET ("binlog.enable" = "false")` | `can't disable binlog when format is
[Row]` |
| `SET ("binlog.format" = "ROW")` (same value) | accepted, no-op |
| `SET ("binlog.ttl_seconds" = "7200")` | accepted |
### Follow-up
There was no regression coverage for this, so I am adding
`regression-test/suites/table_stream_p0/test_binlog_property_alter_exception.groovy`
to lock the behaviour in. It covers `binlog.format`,
`binlog.need_historical_value` and `binlog.enable` on a ROW binlog table, the
no-op case of setting the same value, and the mirror case on a table without
binlog.
One thing worth noting for anyone hitting this again: the whole
`checkBinlogConfigChange` branch is guarded by `Config.enable_feature_binlog`
(`Alter.java:270`). With the feature off the statement is still rejected, but
with `Binlog feature is not enabled` instead. That is not a problem in practice
because table stream consumption already requires `enable_feature_binlog =
true` (`InsertIntoTableCommand.java:296`).
This issue can be closed.
--
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]
