Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion ch_backup/clickhouse/control.py
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,47 @@
"""
)

UNESCAPED_KEYWORDS = [
"TOP",
]


def _fix_create_statement(create_statement: str) -> str:
"""
Fix DDL that was stored by ClickHouse without properly escaping reserved
keywords used as identifiers (column names, aliases).

This is a known ClickHouse bug where keywords like TOP are not backtick-escaped
during DDL serialization to .sql files, making stored DDL unparsable on ATTACH.

Example of broken DDL:
SELECT TOP AS c FROM default.test_3
-- TOP is treated as TOP N syntax

Fixed DDL:
SELECT `TOP` AS c FROM default.test_3
"""

fixed = create_statement
for keyword in UNESCAPED_KEYWORDS:
fixed = re.sub(
rf"(?<![`\w]){keyword}(?![`\w])\s+AS\s",
f"`{keyword}` AS ",
fixed,
flags=re.IGNORECASE,
)

if fixed != create_statement:
logging.warning(
"DDL was modified to fix unescaped reserved keywords. "
"This is likely a ClickHouse bug. "
"Original: {!r}. Fixed: {!r}",
create_statement,
fixed,
)

return fixed


# pylint: disable=too-many-public-methods
class ClickhouseCTL:
Expand Down Expand Up @@ -959,7 +1000,8 @@ def create_table(self, table: Table) -> None:
"""
Restore table.
"""
self._ch_client.query(table.create_statement)
fixed_statement = _fix_create_statement(table.create_statement)
self._ch_client.query(fixed_statement)

def restore_replica(self, table: Table) -> None:
"""
Expand Down
28 changes: 27 additions & 1 deletion tests/unit/test_control.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import pytest

from ch_backup.clickhouse.control import _format_string_array, _parse_version
from ch_backup.clickhouse.control import (
_fix_create_statement,
_format_string_array,
_parse_version,
)
from tests.unit.utils import parametrize


Expand Down Expand Up @@ -49,3 +53,25 @@ def test_format_string_array(value, result):
)
def test_parse_version(version: str, expected: list[int]) -> None:
assert _parse_version(version) == expected


def test_fix_create_statement_top_keyword():
broken = (
"ATTACH VIEW test_4 UUID '1c465905-164d-400b-8a39-ff382c92fb81' "
"(c String) AS WITH A AS (SELECT TOP AS c FROM default.test_3) "
"SELECT c FROM A"
)
fixed = _fix_create_statement(broken)

assert "`TOP`" in fixed
assert "SELECT `TOP` AS c" in fixed
Comment on lines +58 to +67

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Strengthen assertions by checking the full fixed statement rather than only substrings.

Substring checks would still pass if _fix_create_statement made other unintended changes to the DDL. Define a full expected_fixed statement and assert fixed == expected_fixed so the test fully specifies the transformation and better catches regressions.

Suggested change
def test_fix_create_statement_top_keyword():
broken = (
"ATTACH VIEW test_4 UUID '1c465905-164d-400b-8a39-ff382c92fb81' "
"(c String) AS WITH A AS (SELECT TOP AS c FROM default.test_3) "
"SELECT c FROM A"
)
fixed = _fix_create_statement(broken)
assert "`TOP`" in fixed
assert "SELECT `TOP` AS c" in fixed
def test_fix_create_statement_top_keyword() -> None:
broken = (
"ATTACH VIEW test_4 UUID '1c465905-164d-400b-8a39-ff382c92fb81' "
"(c String) AS WITH A AS (SELECT TOP AS c FROM default.test_3) "
"SELECT c FROM A"
)
expected_fixed = (
"ATTACH VIEW test_4 UUID '1c465905-164d-400b-8a39-ff382c92fb81' "
"(c String) AS WITH A AS (SELECT `TOP` AS c FROM default.test_3) "
"SELECT c FROM A"
)
fixed = _fix_create_statement(broken)
assert fixed == expected_fixed



def test_fix_create_statement_no_change_when_valid():
valid = (
"ATTACH VIEW test_4 UUID '1c465905-164d-400b-8a39-ff382c92fb81' "
"(c String) AS SELECT c FROM default.test_3"
)
fixed = _fix_create_statement(valid)

assert fixed == valid