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

Open
#118,349 1 comment 0 reactions 1 assignee Claimed by @jkartseva View on GitHub
comp-database-engines minor potential bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.