Skip to content

Commit

Permalink
Revert "Bug/223 relationship test with limit (#245)" (#247)
Browse files Browse the repository at this point in the history
This reverts commit d8afb93.
  • Loading branch information
genzgd authored Feb 4, 2024
1 parent d8afb93 commit b791bcc
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 49 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#### Bug Fix
- Fixed an issue where Materialize Views would break with a custom schema. Thanks to [Rory Sawyer](https://github.com/SoryRawyer)
for the PR!
- A few tests with LIMIT clause were broken due to parsing error when having settings in the query ([issue](https://github.com/ClickHouse/dbt-clickhouse/issues/223)). We added a dedicated limit placer, that takes into account the settings section (using a comment flag `-- settings_section` within the query).

### Release [1.7.1], 2023-12-13
#### Bug Fixes
Expand Down
19 changes: 2 additions & 17 deletions dbt/adapters/clickhouse/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,30 +412,15 @@ def get_model_settings(self, model):
res = []
for key in settings:
res.append(f' {key}={settings[key]}')
if len(res) == 0:
return ''
else:
settings_str = 'SETTINGS ' + ', '.join(res) + '\n'
return f"""
-- end_of_sql
{settings_str}
"""
return '' if len(res) == 0 else 'SETTINGS ' + ', '.join(res) + '\n'

@available
def get_model_query_settings(self, model):
settings = model['config'].get('query_settings', {})
res = []
for key in settings:
res.append(f' {key}={settings[key]}')

if len(res) == 0:
return ''
else:
settings_str = 'SETTINGS ' + ', '.join(res) + '\n'
return f"""
-- settings_section
{settings_str}
"""
return '' if len(res) == 0 else 'SETTINGS ' + ', '.join(res) + '\n'

@available.parse_none
def get_column_schema_from_query(self, sql: str, *_) -> List[ClickHouseColumn]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ left join parent
on child.from_field = parent.to_field

where parent.to_field is null
-- end_of_sql
settings join_use_nulls = 1

{% endmacro %}
30 changes: 0 additions & 30 deletions dbt/include/clickhouse/macros/utils/utils.sql
Original file line number Diff line number Diff line change
@@ -1,33 +1,3 @@
{% macro clickhouse__get_test_sql(main_sql, fail_calc, warn_if, error_if, limit) -%}
{% set main_sql_formatted = clickhouse__place_limit(main_sql, limit) if limit !=None else main_sql%}
select
{{ fail_calc }} as failures,
{{ fail_calc }} {{ warn_if }} as should_warn,
{{ fail_calc }} {{ error_if }} as should_error
from (
{{ main_sql_formatted }}
) dbt_internal_test

{%- endmacro %}


-- This macro is designed to add a LIMIT clause to a ClickHouse SQL query while preserving any ClickHouse settings specified in the query.
-- When multiple queries are nested, the limit will be attached to the outer query
{% macro clickhouse__place_limit(query, limit) -%}
{% if 'settings' in query.lower()%}
{% if '-- end_of_sql' not in query.lower()%}
{{exceptions.raise_compiler_error("-- end_of_sql must be set when using ClickHouse settings")}}
{% endif %}
{% set split_by_settings_sections = query.split("-- end_of_sql")%}
{% set split_by_settings_sections_with_limit = split_by_settings_sections[-2] + "\n LIMIT " + limit|string + "\n" %}
{% set query_with_limit = "-- end_of_sql".join(split_by_settings_sections[:-2] + [split_by_settings_sections_with_limit, split_by_settings_sections[-1]])%}
{{query_with_limit}}
{% else %}
{{query}}
{{"limit " ~ limit}}
{% endif %}
{%- endmacro %}

{% macro clickhouse__any_value(expression) -%}
any({{ expression }})
{%- endmacro %}
Expand Down

0 comments on commit b791bcc

Please sign in to comment.