Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore/update upstream #1

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8a2731f
Distributed incremental materialization (#172)
gladkikhtutu Jul 27, 2023
3a28a66
Update version and tweak docs
genzgd Jul 27, 2023
1a0649e
Lw delete set fix (#174)
genzgd Jul 27, 2023
4b8a202
Fix legacy incremental materialization (#178)
genzgd Aug 9, 2023
9c8139f
fix: distributed_table materialization issue (#184)
zli06160 Aug 22, 2023
80cba25
Bump version and changelog (#185)
genzgd Aug 22, 2023
b79669a
cluster names containing dash characters (#198) (#200)
the4thamigo-uk Oct 21, 2023
d63285a
Add basic error test, fix minor merge conflict (#202)
genzgd Oct 26, 2023
96474f1
Cluster setting and Distributed Table tests (#186)
gfunc Oct 26, 2023
af72593
Update version and CHANGELOG, incorporate cluster name fix (#203)
genzgd Oct 26, 2023
9edb554
Release 1 5 0 (#210)
genzgd Nov 23, 2023
5e8e54b
Update test and dependency versions. (#211)
genzgd Nov 23, 2023
588e5ca
Adjust the wrapper parenthesis around the table materialization sql c…
kris947 Nov 27, 2023
1f17ec2
Update for 1.5.1 bug fix
genzgd Nov 27, 2023
9997b82
Fix creation of replicated tables when using legacy materialization (…
StevenReitsma Nov 28, 2023
3fec9a4
On cluster sync cleanup
genzgd Nov 28, 2023
bf11cbe
Bug fixes related to model settings. (#214)
genzgd Nov 29, 2023
8561210
Add materialization macro for materialized view (#207)
SoryRawyer Nov 29, 2023
246a4d8
Release 1 6 0 (#215)
genzgd Nov 30, 2023
08bbbf9
Release 1 6 1 (#217)
genzgd Dec 5, 2023
e6e74e4
Release 1 6 2 (#219)
genzgd Dec 6, 2023
2e72a00
Release 1 7 0 (#220)
genzgd Dec 7, 2023
5ccdad5
Correctly warn or error if light weight deletes not available
genzgd Dec 8, 2023
2d5c675
Wrap columns_in_query query in select statement (#222)
ptemarvelde Dec 13, 2023
ca9da0b
Update changelog
genzgd Dec 13, 2023
36ae17e
fix: incremental on cluster clause
Savid Jul 12, 2023
c522eb3
feat(icremental): add distrubted table
Savid Jul 18, 2023
6125272
fix(incremental): replicated delete_insert
Savid Jul 24, 2023
665f184
feat(incremental): distrubted append
Savid Jul 25, 2023
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
Prev Previous commit
Next Next commit
Correctly warn or error if light weight deletes not available
  • Loading branch information
genzgd committed Dec 8, 2023
commit 5ccdad5e80af3d64647a147c01f0e0be33c00485
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
### Release [1.7.1], TBD
#### Bug Fix
- It was possible for incremental models with the delete+insert strategy to fail if ClickHouse "light weight deletes" were
not enabled or the required setting `allow_nondetermistic_mutations` was not enabled and the user did not have permission
to apply it. This condition is now detected on startup, and an exception will be thrown if `use_lw_deletes` is configured
in the profile. Otherwise, a warning will be logged that incremental models will be slower (because such models will
be downgraded to use the `legacy` incremental strategy). This should prevent the confusing behavior in
https://github.com/ClickHouse/dbt-clickhouse/issues/197 by throwing an early exception for an unsupported configuration.

### Release [1.7.0], 2023-12-07
#### Improvements
- Minimal compatibility with dbt 1.7.x. The date_spine macro and additional automated tests have not been implemented,
2 changes: 1 addition & 1 deletion dbt/adapters/clickhouse/__version__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
version = '1.7.0'
version = '1.7.1'
54 changes: 34 additions & 20 deletions dbt/adapters/clickhouse/dbclient.py
Original file line number Diff line number Diff line change
@@ -2,9 +2,15 @@
from abc import ABC, abstractmethod
from typing import Dict

from dbt.exceptions import DbtDatabaseError, FailedToConnectError
from dbt.exceptions import DbtConfigError, DbtDatabaseError, FailedToConnectError

from dbt.adapters.clickhouse.credentials import ClickHouseCredentials
from dbt.adapters.clickhouse.errors import (
lw_deletes_not_enabled_error,
lw_deletes_not_enabled_warning,
nd_mutations_not_enabled_error,
nd_mutations_not_enabled_warning,
)
from dbt.adapters.clickhouse.logger import logger
from dbt.adapters.clickhouse.query import quote_identifier
from dbt.adapters.clickhouse.util import compare_versions
@@ -131,34 +137,42 @@ def update_model_settings(self, model_settings: Dict[str, str]):
model_settings[key] = value

def _check_lightweight_deletes(self, requested: bool):
lw_deletes = self.get_ch_setting(LW_DELETE_SETTING)
nd_mutations = self.get_ch_setting(ND_MUTATION_SETTING)
lw_deletes, lw_read_only = self.get_ch_setting(LW_DELETE_SETTING)
nd_mutations, nd_mutations_read_only = self.get_ch_setting(ND_MUTATION_SETTING)
if lw_deletes is None or nd_mutations is None:
if requested:
logger.warning(
'use_lw_deletes requested but are not available on this ClickHouse server'
)
logger.warning(lw_deletes_not_enabled_error)
return False, False
lw_deletes = int(lw_deletes) > 0
if not lw_deletes:
try:
self.command(f'SET {LW_DELETE_SETTING} = 1')
self._conn_settings[LW_DELETE_SETTING] = '1'
lw_deletes = True
except DbtDatabaseError:
pass
if lw_read_only:
lw_deletes = False
if requested:
raise DbtConfigError(lw_deletes_not_enabled_error)
logger.warning(lw_deletes_not_enabled_warning)
else:
try:
self.command(f'SET {LW_DELETE_SETTING} = 1')
self._conn_settings[LW_DELETE_SETTING] = '1'
lw_deletes = True
except DbtDatabaseError:
logger.warning(lw_deletes_not_enabled_warning)
nd_mutations = int(nd_mutations) > 0
if lw_deletes and not nd_mutations:
try:
self.command(f'SET {ND_MUTATION_SETTING} = 1')
self._conn_settings[ND_MUTATION_SETTING] = '1'
nd_mutations = True
except DbtDatabaseError:
pass
if nd_mutations_read_only:
nd_mutations = False
if requested:
raise DbtConfigError(nd_mutations_not_enabled_error)
logger.warning(nd_mutations_not_enabled_warning)
else:
try:
self.command(f'SET {ND_MUTATION_SETTING} = 1')
self._conn_settings[ND_MUTATION_SETTING] = '1'
nd_mutations = True
except DbtDatabaseError:
logger.warning(nd_mutations_not_enabled_warning)
if lw_deletes and nd_mutations:
return True, requested
if requested:
logger.warning('use_lw_deletes requested but cannot enable on this ClickHouse server')
return False, False

def _ensure_database(self, database_engine, cluster_name) -> None:
21 changes: 21 additions & 0 deletions dbt/adapters/clickhouse/errors.py
Original file line number Diff line number Diff line change
@@ -22,3 +22,24 @@

Source columns not in target: {0}
"""

lw_deletes_not_enabled_error = """
Attempting to apply the configuration `use_lw_deletes` to enable the delete+insert incremental strategy, but
`light weight deletes` are either not available or not enabled on this ClickHouse server.
"""

lw_deletes_not_enabled_warning = """
`light weight deletes` are either not available or not enabled on this ClickHouse server. This prevents the use
of the delete+insert incremental strategy, which may negatively affect performance for incremental models.
"""

nd_mutations_not_enabled_error = """
Attempting to apply the configuration `use_lw_deletes` to enable the delete+insert incremental strategy, but
the required `allow_nondeterministic_mutations` is not enabled and is `read_only` for this user
"""

nd_mutations_not_enabled_warning = """
The setting `allow_nondeterministic_mutations` is not enabled and is `read_only` for this user` This prevents the use
of `light weight deletes` and therefore the delete+insert incremental strategy. This may negatively affect performance
for incremental models
"""
2 changes: 1 addition & 1 deletion dbt/adapters/clickhouse/httpclient.py
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@ def columns_in_query(self, sql: str, **kwargs) -> List[ClickHouseColumn]:

def get_ch_setting(self, setting_name):
setting = self._client.server_settings.get(setting_name)
return setting.value if setting else None
return (setting.value, setting.readonly) if setting else (None, 0)

def database_dropped(self, database: str):
# This is necessary for the http client to avoid exceptions when ClickHouse doesn't recognize the database
4 changes: 2 additions & 2 deletions dbt/adapters/clickhouse/nativeclient.py
Original file line number Diff line number Diff line change
@@ -42,12 +42,12 @@ def columns_in_query(self, sql: str, **kwargs) -> List[ClickHouseColumn]:
def get_ch_setting(self, setting_name):
try:
result = self._client.execute(
f"SELECT value FROM system.settings WHERE name = '{setting_name}'"
f"SELECT value, readonly FROM system.settings WHERE name = '{setting_name}'"
)
except clickhouse_driver.errors.Error as ex:
logger.warn('Unexpected error retrieving ClickHouse server setting', ex)
return None
return result[0][0] if result else None
return (result[0][0], result[0][1]) if result else (None, 0)

def close(self):
self._client.disconnect()