morningman opened a new pull request, #67471:
URL: https://github.com/apache/doris/pull/67471
### What problem does this PR solve?
Issue Number: close #65388
Related PR: #65810
Problem Summary:
**1. `ALTER STREAM ... SET COMMENT` was not supported (#65388)**
A table stream can be created with a comment and the comment is fully wired
up everywhere except for changing it:
| step | before this PR |
| --- | --- |
| `CREATE STREAM s ON TABLE t COMMENT 'x'` | supported
(`InternalCatalog#createTableStream`) |
| persisted in the image | supported (`Table#comment`) |
| `SHOW CREATE STREAM s` | prints the comment |
| `information_schema.table_streams.STREAM_COMMENT` | exposes the comment |
| changing the comment | **not possible** |
There was no `ALTER STREAM` rule in `DorisParser.g4` at all — `STREAM` only
appeared in `CREATE STREAM`, `DROP STREAM`, `SHOW STREAMS` and `SHOW CREATE
STREAM` — so the statement failed at parser stage:
```
errCode = 2, detailMessage = no viable alternative at input 'ALTER
STREAM'(line 1, pos 6)
```
`ALTER TABLE` is not an alternative either: `Alter#processAlterTable`
rejects the `STREAM` table type with `Do not support alter STREAM table[...]`.
This PR adds:
```sql
ALTER STREAM <name> SET COMMENT 'new comment';
ALTER STREAM <name> MODIFY COMMENT 'new comment'; -- same thing
```
`MODIFY` is accepted alongside `SET` so the syntax stays consistent with
`ALTER TABLE ... MODIFY COMMENT`, which is the existing Doris spelling for the
same operation on a table.
Implementation notes:
- The comment of a stream lives in the `Table` metadata only, so
`Alter#processAlterStreamComment` reuses
`ModifyCommentOperationLog.forTable(...)` and the existing replay path
`Alter#replayModifyComment`, which already resolves a generic `Table`. **No new
edit log operation and no meta version bump.**
- Cloud Meta Service only stores stream offsets and ids
(`CloudInternalCatalog#beforeCreateTableStream` / `#afterCreateTableStream`),
so no extra RPC is needed and the behaviour is the same in cloud mode.
- `AlterStreamCommand` extends `AlterCommand`, which already provides
`ForwardWithSync` and `StmtType.ALTER`. It carries an `AlterType` enum so that
other `ALTER STREAM` clauses can be added later without reshaping the command.
- Privilege required is `ALTER` on the stream, matching `ALTER TABLE`.
Altering a non-stream table through `ALTER STREAM` reports `ERR_WRONG_OBJECT`,
the same way `SHOW CREATE STREAM` does.
- `Config.enable_table_stream` gates the operation, consistent with `CREATE
STREAM` and `DROP STREAM`.
**2. Regression coverage for immutable binlog properties (#65383)**
`ALTER TABLE ... SET ("binlog.format" = ...)` on a ROW binlog table used to
fail with a misleading light-schema-change error, because
`AlterOperations#checkBinlogConfigChange` did not list `binlog.format` /
`binlog.need_historical_value` and the statement was dispatched to the generic
schema change path. That was fixed as a side effect of #65810 (`f745ddf9e22`),
but no test locked the behaviour in. This PR adds
`test_binlog_property_alter_exception.groovy` covering:
| statement (on a `binlog.format = ROW` MOW table) | expected |
| --- | --- |
| `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 |
| `SET ("binlog.format" = "ROW")` on a table without binlog | `not support
change binlog format from STATEMENT_AND_SNAPSHOT to ROW` |
### Release note
Support `ALTER STREAM <name> SET|MODIFY COMMENT '<comment>'` to change the
comment of a table stream.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
-
`regression-test/suites/table_stream_p0/test_table_stream_alter_comment.groovy`
-
`regression-test/suites/table_stream_p0/test_binlog_property_alter_exception.groovy`
- [x] Unit Test
-
`fe/fe-core/src/test/java/org/apache/doris/catalog/AlterTableStreamCommentTest.java`
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. A new statement `ALTER STREAM ... SET|MODIFY COMMENT` is
accepted. It was a parser error before, so no existing statement changes
behaviour.
- Does this need documentation?
- [ ] No.
- [x] Yes. A doc PR for the new statement will be filed against
apache/doris-website.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Xx7TgjXJCiChnzLYa6hgtL
--
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]
