Skip to content

Commit

Permalink
Review fix
Browse files Browse the repository at this point in the history
  • Loading branch information
MikhailBurdukov committed Jul 16, 2024
1 parent f00935b commit c037445
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
7 changes: 6 additions & 1 deletion ch_backup/clickhouse/control.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
SELECT
database,
name,
create_table_query,
engine,
create_table_query
FROM system.tables
Expand Down Expand Up @@ -566,6 +565,12 @@ def get_table(
Get table by name, returns None if no table has found.
"""
tables = self.get_tables(db_name, [table_name], short_query)

if len(tables) > 1:
raise RuntimeError(
f"Found several tables, when expected to find single table: database {db_name}, table {table_name}"
)

return tables[0] if len(tables) == 1 else None

def does_table_exist(self, db_name: str, table_name: str) -> bool:
Expand Down
4 changes: 2 additions & 2 deletions ch_backup/clickhouse/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,14 @@ def is_replicated(self) -> bool:
"""
Return True if table engine belongs to replicated merge tree table engine family, or False otherwise.
"""
return self.is_merge_tree() and self.engine.find("Replicated") != -1
return Table.engine_is_replicated(self.engine)

@staticmethod
def engine_is_replicated(engine: str) -> bool:
"""
A static method for determining whether an engine is replicated or not.
"""
return Table("", "", engine, [], [], "", "", None).is_replicated()
return "MergeTree" in engine and "Replicated" in engine

def is_merge_tree(self) -> bool:
"""
Expand Down
9 changes: 5 additions & 4 deletions ch_backup/logic/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -651,16 +651,17 @@ def _restore_data(
maybe_table_short = context.ch_ctl.get_table(
table_meta.database, table_meta.name, short_query=True
)
assert (
maybe_table_short is not None
), f"Table not found {table_meta.database}.{table_meta.name}"
if not maybe_table_short:
raise ClickhouseBackupError(
f"Table not found {table_meta.database}.{table_meta.name}"
)

# We have to check table engine on short Table version
# because some of columns might be inaccessbible, for old ch versions.
# Fix https://github.com/ClickHouse/ClickHouse/pull/55540 is pesented since 23.8.
if not maybe_table_short.is_merge_tree():
logging.debug(
'Skiptable "{}.{}" data restore, because it is not MergeTree like.',
'Skip table "{}.{}" data restore, because it is not MergeTree family.',
table_meta.database,
table_meta.name,
)
Expand Down

0 comments on commit c037445

Please sign in to comment.