fix error on complex delete_all (#255)

* reproduce

* fix?

Author: ruslandoga

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- Fix `delete_all` with subqueries https://github.com/plausible/ecto_ch/pull/255
+
 ## 0.8.2 (2025-08-26)
 
 - Add `Ecto.Query.API.splice/1` support https://github.com/plausible/ecto_ch/pull/240

lib/ecto/adapters/clickhouse/connection.ex

@@ -125,7 +125,7 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
     where =
       case query.wheres do
         [] -> " WHERE 1"
-        _ -> where(query, {{nil, nil, nil}}, params)
+        _ -> where(query, {{nil, nil, nil}, []}, params)
       end
 
     ["ALTER TABLE ", quote_table(prefix, table), " UPDATE ", fields, where]
@@ -196,7 +196,7 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
     where =
       case query.wheres do
         [] -> " WHERE 1"
-        _ -> where(query, {{nil, nil, nil}}, params)
+        _ -> where(query, {{nil, nil, nil}, []}, params)
       end
 
     ["DELETE FROM ", quote_table(prefix, table) | where]

test/ecto/adapters/clickhouse/connection_test.exs

@@ -1472,6 +1472,28 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
     assert delete_all(query) == ~s{DELETE FROM "first"."schema" WHERE 1}
   end
 
+  # https://github.com/plausible/ecto_ch/issues/247
+  test "delete all with subquery" do
+    hashes =
+      from uim in "user_id_map",
+        where: uim.account_id == ^91241 and like(uim.user_id, "anon:%"),
+        select: uim.user_id_hash
+
+    query =
+      from rup in "recent_user_profiles",
+        where: rup.account_id == ^91241 and rup.user_id_hash in subquery(hashes)
+
+    assert delete_all(query) == """
+           DELETE FROM "recent_user_profiles" \
+           WHERE (\
+           ("account_id" = {$0:Int64}) AND \
+           ("user_id_hash" IN (\
+           SELECT su0."user_id_hash" FROM "user_id_map" AS su0 \
+           WHERE (\
+           (su0."account_id" = {$1:Int64}) AND (su0."user_id" LIKE 'anon:%')))))\
+           """
+  end
+
   test "CTE alter_update_all" do
     cte_query =
       from(x in Schema,
@@ -1513,6 +1535,28 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
                  fn -> alter_update_all(query) end
   end
 
+  test "alter_update_all with subquery" do
+    hashes =
+      from uim in "user_id_map",
+        where: uim.account_id == ^91241 and like(uim.user_id, "anon:%"),
+        select: uim.user_id_hash
+
+    query =
+      from rup in "recent_user_profiles",
+        where: rup.account_id == ^91241 and rup.user_id_hash in subquery(hashes),
+        update: [set: [timestamp: ^0]]
+
+    assert alter_update_all(query) == """
+           ALTER TABLE "recent_user_profiles" UPDATE "timestamp"={$0:Int64} \
+           WHERE (\
+           ("account_id" = {$1:Int64}) AND \
+           ("user_id_hash" IN (\
+           SELECT su0."user_id_hash" FROM "user_id_map" AS su0 \
+           WHERE (\
+           (su0."account_id" = {$2:Int64}) AND (su0."user_id" LIKE 'anon:%')))))\
+           """
+  end
+
   # TODO alter_delete_all
 
   describe "windows" do

test/ecto/integration/delete_all_test.exs

@@ -0,0 +1,105 @@
+defmodule Ecto.Integration.DeleteAllTest do
+  use Ecto.Integration.Case
+  import Ecto.Query
+  alias Ecto.Integration.TestRepo
+
+  @moduletag :lightweight_delete
+
+  # based on https://github.com/plausible/ecto_ch/issues/247
+
+  setup do
+    TestRepo.query!("""
+    CREATE TABLE user_id_map (
+      account_id UInt64,
+      user_id_hash UInt64,
+      user_id String
+    ) ENGINE ReplacingMergeTree() ORDER BY (account_id, user_id_hash)
+    """)
+
+    on_exit(fn -> TestRepo.query!("DROP TABLE user_id_map") end)
+
+    TestRepo.query!("""
+    CREATE TABLE recent_user_profiles (
+      account_id UInt64,
+      user_id_hash UInt64,
+      timestamp DateTime64(3)
+    ) ENGINE ReplacingMergeTree(timestamp)
+    ORDER BY (account_id, user_id_hash)
+    SETTINGS enable_block_number_column = 1, enable_block_offset_column = 1
+    """)
+
+    on_exit(fn -> TestRepo.query!("DROP TABLE recent_user_profiles") end)
+
+    :ok
+  end
+
+  defmodule UserIdMap do
+    use Ecto.Schema
+
+    @primary_key false
+    schema "user_id_map" do
+      field :account_id, Ch, type: "UInt64"
+      field :user_id_hash, Ch, type: "UInt64"
+      field :user_id, :string
+    end
+  end
+
+  defmodule RecentUserProfiles do
+    use Ecto.Schema
+
+    @primary_key false
+    schema "recent_user_profiles" do
+      field :account_id, Ch, type: "UInt64"
+      field :user_id_hash, Ch, type: "UInt64"
+      field :timestamp, Ch, type: "DateTime64(3)"
+    end
+  end
+
+  test "delete_all with subqeury" do
+    TestRepo.insert!(%UserIdMap{account_id: 91241, user_id_hash: 100, user_id: "anon:123"})
+    TestRepo.insert!(%UserIdMap{account_id: 91241, user_id_hash: 200, user_id: "registered:bob"})
+
+    TestRepo.insert!(%RecentUserProfiles{
+      account_id: 91241,
+      user_id_hash: 100,
+      timestamp: ~N[2023-01-01 10:00:00.000000]
+    })
+
+    TestRepo.insert!(%RecentUserProfiles{
+      account_id: 91241,
+      user_id_hash: 200,
+      timestamp: ~N[2023-01-01 12:00:00.000000]
+    })
+
+    list_recent_user_profiles = fn ->
+      TestRepo.all(
+        from rup in RecentUserProfiles,
+          inner_join: uim in UserIdMap,
+          on: uim.user_id_hash == rup.user_id_hash,
+          order_by: [asc: :timestamp],
+          select: uim.user_id
+      )
+    end
+
+    assert list_recent_user_profiles.() == ["anon:123", "registered:bob"]
+
+    hashes =
+      from uim in UserIdMap,
+        where: uim.account_id == ^91241 and like(uim.user_id, "anon:%"),
+        select: uim.user_id_hash
+
+    query =
+      from rup in RecentUserProfiles,
+        where: rup.account_id == ^91241 and rup.user_id_hash in subquery(hashes)
+
+    TestRepo.delete_all(query,
+      settings: [
+        lightweight_deletes_sync: 0,
+        lightweight_delete_mode: "lightweight_update_force",
+        alter_update_mode: "lightweight"
+      ]
+    )
+
+    assert list_recent_user_profiles.() == ["registered:bob"]
+  end
+end

test/test_helper.exs

@@ -52,7 +52,7 @@ exclude =
     []
   else
     # these types are not supported in older ClickHouse versions we have in the CI
-    [:time, :variant, :json, :dynamic]
+    [:time, :variant, :json, :dynamic, :lightweight_delete]
   end
 
 {:ok, _} = Ecto.Adapters.ClickHouse.ensure_all_started(TestRepo.config(), :temporary)