From 3fec9a4e669a6032c8ec712391812e897ff326c3 Mon Sep 17 00:00:00 2001
From: Geoff Genz <geoff@clickhouse.com>
Date: Tue, 28 Nov 2023 08:48:34 -0700
Subject: [PATCH] On cluster sync cleanup

---
 .github/workflows/test_matrix.yml                        | 1 +
 CHANGELOG.md                                             | 7 +++++++
 dbt/adapters/clickhouse/__version__.py                   | 2 +-
 dbt/include/clickhouse/macros/adapters.sql               | 2 +-
 dbt/include/clickhouse/macros/materializations/table.sql | 5 ++++-
 5 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/test_matrix.yml b/.github/workflows/test_matrix.yml
index 636b2a71..3da76748 100644
--- a/.github/workflows/test_matrix.yml
+++ b/.github/workflows/test_matrix.yml
@@ -66,6 +66,7 @@ jobs:
       - name: Run Native tests
         env:
           DBT_CH_TEST_PORT: 9000
+          DBT_CH_TEST_CLUSTER: test_shard
         run: |
           PYTHONPATH="${PYTHONPATH}:dbt"
           pytest tests
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f8b1f0ac..bd0431d3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+### Release [1.5.2], 2023-11-28
+#### Bug Fix
+- The `ON CLUSTER` clause was in the incorrect place for legacy incremental materializations.  This has been fixed.  Thanks to
+[Steven Reitsma](https://github.com/StevenReitsma) for the fix!
+- The `ON CLUSTER` DDL for drop tables did not include a SYNC modifier, which might be the cause of some "table already exists"
+errors
+
 ### Release [1.5.1], 2023-11-27
 #### Bug Fix
 - Fix table materialization for compatibility with SQLFluff.  Thanks to [Kristof Szaloki](https://github.com/kris947) for the PR!
diff --git a/dbt/adapters/clickhouse/__version__.py b/dbt/adapters/clickhouse/__version__.py
index b9148ac4..e8b09c2b 100644
--- a/dbt/adapters/clickhouse/__version__.py
+++ b/dbt/adapters/clickhouse/__version__.py
@@ -1 +1 @@
-version = '1.5.1'
+version = '1.5.2'
diff --git a/dbt/include/clickhouse/macros/adapters.sql b/dbt/include/clickhouse/macros/adapters.sql
index ae0ef8d6..718c775a 100644
--- a/dbt/include/clickhouse/macros/adapters.sql
+++ b/dbt/include/clickhouse/macros/adapters.sql
@@ -56,7 +56,7 @@
 {% macro clickhouse__drop_relation(relation, obj_type='table') -%}
   {% call statement('drop_relation', auto_begin=False) -%}
     {# drop relation on cluster by default if cluster is set #}
-    drop {{ obj_type }} if exists {{ relation }} {{ on_cluster_clause(relation.without_identifier())}}
+    drop {{ obj_type }} if exists {{ relation }} {{ on_cluster_clause(relation.without_identifier(), True)}}
   {%- endcall %}
 {% endmacro %}
 
diff --git a/dbt/include/clickhouse/macros/materializations/table.sql b/dbt/include/clickhouse/macros/materializations/table.sql
index 6282ad40..ca07cdbe 100644
--- a/dbt/include/clickhouse/macros/materializations/table.sql
+++ b/dbt/include/clickhouse/macros/materializations/table.sql
@@ -121,11 +121,14 @@
   {%- endif %}
 {%- endmacro -%}
 
-{% macro on_cluster_clause(relation) %}
+{% macro on_cluster_clause(relation, force_sync) %}
   {% set active_cluster = adapter.get_clickhouse_cluster_name() %}
   {%- if active_cluster is not none and relation.should_on_cluster %}
     {# Add trailing whitespace to avoid problems when this clause is not last #}
     ON CLUSTER {{ active_cluster + ' ' }}
+    {%- if force_sync %}
+    SYNC
+    {%- endif %}
   {%- endif %}
 {%- endmacro -%}