ClickHouse / ClickHouse/ClickHouse
`ALTER DATABASE ... MODIFY COMMENT` on a `Backup` database rewrites its metadata with a string-literal locator, and the server refuses to start
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
### Company or project name
ClickHouse
### Describe what's wrong
`ALTER DATABASE ... MODIFY COMMENT` on a database with `ENGINE = Backup(...)` rewrites the database's metadata file with the backup locator serialized as a string literal instead of a function, and the engine refuses to parse that form back. The next server restart aborts during metadata loading with `BAD_ARGUMENTS`, and the server stays down on every subsequent start until the operator hand-edits or deletes `metadata/.sql` out-of-band. A single routine comment change on one read-only database therefore takes the whole server out.
The round trip that corrupts the file: `DatabaseBackup::getCreateDatabaseQueryImpl` regenerates the definition as `Backup('', '')` — the second argument is `quoteString(config.backup_info.toString())`, a string literal. `alterDatabaseComment` persists exactly this AST via `DatabaseCatalog::updateMetadataFile`. On the next startup, `parseArguments` in `DatabaseBackup.cpp` feeds that argument to `BackupInfo::fromAST`, which accepts only a function and throws `Expected function, got 'File(...)'`; `loadDatabase` rethrows, so startup fails before the server accepts connections — no DDL can be issued to repair or drop the database.
Note that a `BackupInfo::fromString` helper that parses exactly this string form already exists, and the load path could fall back to it (or the regeneration could emit the function form via `BackupInfo::toAST`). The in-flight #117429 adds a normalizer for this legacy string-literal form on the restore-from-backup path (`BackupMetadataFinder`), but the database metadata-load path is not covered by it, so the startup failure remains. Related but distinct: #82112 is about startup failure when the backup itself is missing; here the backup is intact and the failure is in parsing the rewritten metadata, before the backup is even opened.
### Does it reproduce on the most recent release?
Yes — reproduced on 26.8.2.7 (LTS) and on a current 26.9.1.1 development build. A restart of the same data directory without the `MODIFY COMMENT` step comes up cleanly, so the comment change is the only trigger.
### How to reproduce
Server config needs a backup destination, e.g.:
```xml
/data/backups
```
```sql
CREATE TABLE default.src_t (uid Int16, name String) ENGINE = MergeTree ORDER BY uid;
INSERT INTO default.src_t VALUES (1, 'a'), (2, 'b'), (3, 'c');
BACKUP TABLE default.src_t TO File('/data/backups/bk1');
CREATE DATABASE bdb ENGINE = Backup('default', File('/data/backups/bk1'));
SELECT count() FROM bdb.src_t; -- 3, the database works
ALTER DATABASE bdb MODIFY COMMENT 'routine note';
```
`metadata/bdb.sql` before the `ALTER`:
```sql
ATTACH DATABASE bdb
ENGINE = Backup('default', File('/data/backups/bk1'))
```
after the `ALTER` (the locator is now a quoted string):
```sql
ATTACH DATABASE bdb
ENGINE = Backup('default', 'File(\'/data/backups/bk1\')')
COMMENT 'routine note'
```
Restart the server: it fails to start, and keeps failing on every retry.
### Expected behavior
`ALTER DATABASE ... MODIFY COMMENT` persists a definition that the server can load back, and the server starts normally after a restart with the `Backup` database attached and readable.
### Error message and/or stacktrace
```
Application: Code: 36. DB::Exception: Expected function, got 'File(\'/data/backups/bk1\')': while loading database `bdb` from file metadata/bdb.sql. (BAD_ARGUMENTS)
4. DB::BackupInfo::fromAST(DB::IAST const&) @ 0x000000001773f9d0
5. src/Databases/DatabaseBackup.cpp:505:26: DB::(anonymous namespace)::parseArguments(...)
6. src/Databases/DatabaseBackup.cpp:534:23: ... registerDatabaseBackup ...
```
### Additional context
`SHOW CREATE DATABASE bdb` displays the same regenerated string-literal form, which is how the corrupted shape can be spotted before a restart. Recovery without data loss is possible only by editing `metadata/.sql` by hand to turn the quoted locator back into a function call (or by deleting the file) while the server is down.
Contributor guide
Assessment
This issue has not been assessed yet.