Skip to content

Commit

Permalink
fix: engin bug
Browse files Browse the repository at this point in the history
    1. migrate ReplacingMergeTree with [`ver`](https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replacingmergetree#ver) raising `AttributeError: 'F' object has no attribute 'get_source_expressions'`.
    2.  unable to omit [`zoo_path`](https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replication#zoo_path) and [`replica_name`](https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replication#replica_name) in replicated engines other than `ReplicatedMergeTree`.
  • Loading branch information
jayvynl committed Feb 21, 2024
1 parent a946b21 commit 2065bed
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 63 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
### 1.1.7

- fix: #76 `AttributeError: 'ReplicatedReplacingMergeTree' object has no attribute 'expressions'`.
- fix: migrate ReplacingMergeTree with [`ver`](https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replacingmergetree#ver) raising `AttributeError: 'F' object has no attribute 'get_source_expressions'`.
- fix: unable to omit [`zoo_path`](https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replication#zoo_path) and [`replica_name`](https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replication#replica_name) in replicated engines other than `ReplicatedMergeTree`.

### 1.1.6

- add `CLICKHOUSE_ENABLE_UPDATE_ROWCOUNT` django setting.
Expand Down
2 changes: 1 addition & 1 deletion clickhouse_backend/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from clickhouse_backend.utils.version import get_version

VERSION = (1, 1, 6, "final", 0)
VERSION = (1, 1, 7, "final", 0)

__version__ = get_version(VERSION)
6 changes: 5 additions & 1 deletion clickhouse_backend/backend/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
Table,
)
from django.db.models.constraints import CheckConstraint, UniqueConstraint
from django.db.models.expressions import ExpressionList
from django.db.models.expressions import Col, ExpressionList, F
from django.db.models.indexes import IndexExpression

from clickhouse_backend import compat
Expand Down Expand Up @@ -166,6 +166,10 @@ def _get_engine_expression(self, model, engine):
compiler = Query(model, alias_cols=False).get_compiler(
connection=self.connection
)
engine.source_expressions = tuple(
Col("", model._meta.get_field(e.name)) if isinstance(e, F) else e
for e in engine.source_expressions
)
return Expressions(model._meta.db_table, engine, compiler, self.quote_value)

def column_sql(self, model, field, include_default=False):
Expand Down
74 changes: 23 additions & 51 deletions clickhouse_backend/models/engines.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,63 +185,35 @@ class VersionedCollapsingMergeTree(BaseMergeTree):


class GraphiteMergeTree(BaseMergeTree):
arity = 1

def __init__(self, *expressions, **extra):
if expressions:
expressions = (value_if_string(expressions[0]), *expressions[1:])
super().__init__(*expressions, **extra)
def __init__(self, config_section, **extra):
super().__init__(value_if_string(config_section), **extra)


class ReplicatedMixin:
def __init__(self, *expressions, **extra):
if self.arity is not None and len(expressions) != self.arity + 2:
raise TypeError(
"'%s' takes exactly %s arguments (%s given)"
% (
self.__class__.__name__,
self.arity + 2,
len(expressions),
)
)
if self.arity is None and len(expressions) < 2:
raise TypeError(
"'%s' takes at least 2 arguments (%s given)"
% (
self.__class__.__name__,
len(expressions),
)
)
if self.max_arity is not None and len(expressions) > self.max_arity + 2:
raise TypeError(
"'%s' takes at most %s arguments (%s given)"
% (
self.__class__.__name__,
self.max_arity + 2,
len(expressions),
def __init__(self, *replicated_parameters, other_parameters=(), **extra):
"""
https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replication#replicatedmergetree-parameters
https://github.com/ClickHouse/ClickHouse/issues/8675
https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replication#creating-replicated-tables
replicated_parameters: zoo_path and replica_name. Their default values can be specified in the server
configuration file, in this case, they both can be omitted.
other_parameters: Parameters of an engine which is used for creating the replicated version,
for example, version in ReplacingMergeTree
"""
if replicated_parameters:
if len(replicated_parameters) != 2:
raise TypeError(
"'ReplicatedMergeTree' takes 0 or 2 arguments (%s given)"
% len(replicated_parameters)
)
)
replicated_params = map(value_if_string, expressions[:2])
super().__init__(*expressions[2:], **extra)
self.source_expressions = (*replicated_params, *self.source_expressions)

replicated_parameters = map(value_if_string, replicated_parameters)
super().__init__(*other_parameters, **extra)
self.source_expressions = (*replicated_parameters, *self.source_expressions)

class ReplicatedMergeTree(MergeTree):
arity = None

def __init__(self, *expressions, **extra):
# https://github.com/ClickHouse/ClickHouse/issues/8675
# https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replication#creating-replicated-tables
# You can specify default arguments for Replicated table engine in the server configuration file.
# In this case, you can omit arguments when creating tables.
if expressions:
if len(expressions) != 2:
raise TypeError(
"'ReplicatedMergeTree' takes at 0 or 2 arguments (%s given)"
% len(expressions)
)
expressions = map(value_if_string, expressions)
super().__init__(*expressions, **extra)
class ReplicatedMergeTree(ReplicatedMixin, MergeTree):
pass


class ReplicatedReplacingMergeTree(ReplicatedMixin, ReplacingMergeTree):
Expand Down
32 changes: 32 additions & 0 deletions tests/clickhouse_table_engine/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,35 @@ class Meta:
"cluster", models.currentDatabase(), Student._meta.db_table, models.Rand()
)
cluster = "cluster"


class ReplacingMergeTree(models.ClickhouseModel):
ver = models.UInt64Field()
is_deleted = models.UInt8Field()

class Meta:
engine = models.ReplacingMergeTree("ver", "is_deleted", order_by="id")


class ReplicatedReplacingMergeTree(models.ClickhouseModel):
ver = models.UInt64Field()
is_deleted = models.UInt8Field()

class Meta:
engine = models.ReplicatedReplacingMergeTree(
other_parameters=("ver", "is_deleted"), order_by="id"
)
cluster = "cluster"


class ReplicatedReplacingMergeTreeWithZooReplica(models.ClickhouseModel):
ver = models.UInt64Field()
is_deleted = models.UInt8Field()

class Meta:
engine = models.ReplicatedReplacingMergeTree(
"/clickhouse/tables/{shard}/table_name",
"{replica}",
order_by="id",
)
cluster = "cluster"
36 changes: 26 additions & 10 deletions tests/clickhouse_table_engine/tests.py
Original file line number Diff line number Diff line change
@@ -1,45 +1,61 @@
from django.db import connection
from django.test import TestCase

from clickhouse_backend import models
from clickhouse_backend.models import MergeTree
from clickhouse_backend.utils.timezone import get_timezone

from .models import EngineWithSettings, Event
from . import models


class TestMergeTree(TestCase):
def test_table(self):
opts = Event._meta
def assertEngineEquals(self, model, engine):
with connection.cursor() as cursor:
cursor.execute(
f"select engine_full from system.tables where table='{opts.db_table}'"
f"select engine_full from system.tables where table='{model._meta.db_table}'"
)
engine_full = cursor.fetchone()[0]
self.assertEqual(
engine_full.partition(" SETTINGS ")[0],
engine,
)

def test_table(self):
self.assertEngineEquals(
models.Event,
f"MergeTree PARTITION BY toYYYYMMDD(timestamp, '{get_timezone()}') PRIMARY KEY timestamp ORDER BY (timestamp, id)",
)
self.assertEngineEquals(
models.ReplacingMergeTree, "ReplacingMergeTree(ver, is_deleted) ORDER BY id"
)
self.assertEngineEquals(
models.ReplicatedReplacingMergeTree,
"ReplicatedReplacingMergeTree('/clickhouse/tables/{uuid}/{shard}', '{replica}', ver, is_deleted) ORDER BY id",
)
self.assertEngineEquals(
models.ReplicatedReplacingMergeTreeWithZooReplica,
"ReplicatedReplacingMergeTree('/clickhouse/tables/{shard}/table_name', '{replica}') ORDER BY id",
)

def test_mergetree_init_exception(self):
with self.assertRaisesMessage(
AssertionError, "At least one of order_by or primary_key must be provided"
):
models.MergeTree()
MergeTree()
with self.assertRaisesMessage(ValueError, "None is not allowed in order_by"):
models.MergeTree(order_by=(None, "a"))
MergeTree(order_by=(None, "a"))
with self.assertRaisesMessage(
ValueError, "primary_key must be a prefix of order_by"
):
models.MergeTree(order_by=("a", "b"), primary_key=["b"])
MergeTree(order_by=("a", "b"), primary_key=["b"])
with self.assertRaisesMessage(
ValueError, "primary_key must be a prefix of order_by"
):
models.MergeTree(order_by=("a", "b"), primary_key=["a", "b", "c"])
MergeTree(order_by=("a", "b"), primary_key=["a", "b", "c"])


class TestEngineSettings(TestCase):
def test(self):
opts = EngineWithSettings._meta
opts = models.EngineWithSettings._meta
with connection.cursor() as cursor:
cursor.execute(
f"select engine_full from system.tables where table='{opts.db_table}'"
Expand Down

0 comments on commit 2065bed

Please sign in to comment.