From 462a6620d8a9010fa16a225f40bc5c706b3fc7f2 Mon Sep 17 00:00:00 2001
From: Vasiliy Koshkin <61789920+melancholictheory@users.noreply.github.com>
Date: Tue, 29 Sep 2026 19:41:15 +0300
Subject: [PATCH 1/2] fix(sync): tolerate non-numeric shard and replica names
 in stale replica cleanup

listStaleDatabaseReplicasQuery cast database_shard_name and
database_replica_name from system.clusters with toInt32. That table lists
every cluster the server knows about: the static clusters from
remote_servers carry empty names and a Replicated database created by a
user carries whatever names the user chose. Neither parses as an integer,
the cast throws, CleanupDatabaseReplicas fails, and reconciliation loops
on SchemaInSync=ReplicasNotCleanedUp without starting new pods.

Cast with toInt32OrNull and drop rows where either name is NULL. Rows the
operator did not number were never something the cleanup could act on, so
the remaining logic sees exactly the rows it saw before. The
database_replica_name != '' guard folds into the NULL filter.

The commander integration suite gets a spec that creates a Replicated
database with shard-a/replica-b names and runs the cleanup; it fails on the
cast without this change.

Fixes #349
---
 internal/controller/clickhouse/commands.go      | 10 +++++++---
 internal/controller/clickhouse/commands_test.go | 16 ++++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/internal/controller/clickhouse/commands.go b/internal/controller/clickhouse/commands.go
index 409602cee..2f65f2643 100644
--- a/internal/controller/clickhouse/commands.go
+++ b/internal/controller/clickhouse/commands.go
@@ -23,6 +23,10 @@ WHERE
 	engine NOT IN ('Atomic', 'Lazy', 'SQLite', 'Ordinary', 'Memory')
 SETTINGS
 	format_display_secrets_in_show_and_select=1`
+	// system.clusters also lists static clusters from remote_servers and Replicated databases with
+	// names the operator did not choose; their shard and replica names are empty or non-numeric.
+	// toInt32OrNull keeps the cast from throwing on those rows, and the NULL filter drops them, so
+	// only replicas that the operator itself numbered are considered for cleanup.
 	listStaleDatabaseReplicasQuery = `SELECT
 	database,
 	shard_id,
@@ -31,11 +35,11 @@ SETTINGS
 FROM (
 	SELECT
 		cluster as database,
-		toInt32(database_shard_name) AS shard_id,
-		toInt32(database_replica_name) AS replica_id,
+		toInt32OrNull(database_shard_name) AS shard_id,
+		toInt32OrNull(database_replica_name) AS replica_id,
 		is_active
 	FROM clusterAllReplicas(default, system.clusters)
-	WHERE database_replica_name != '' AND (shard_id >= ? OR replica_id >= ?)
+	WHERE shard_id IS NOT NULL AND replica_id IS NOT NULL AND (shard_id >= ? OR replica_id >= ?)
 )
 GROUP BY database, shard_id, replica_id
 SETTINGS skip_unavailable_shards=1`
diff --git a/internal/controller/clickhouse/commands_test.go b/internal/controller/clickhouse/commands_test.go
index b06fa0370..5475facf5 100644
--- a/internal/controller/clickhouse/commands_test.go
+++ b/internal/controller/clickhouse/commands_test.go
@@ -431,6 +431,22 @@ var _ = Describe("commander", Ordered, Label("integration"), func() {
 			Expect(cmd.CleanupDatabaseReplicas(ctx, cmd.log, map[v1.ClickHouseReplicaID]struct{}{})).To(Succeed())
 		})
 
+		It("should ignore replicated databases whose shard and replica names the operator did not choose", func(ctx context.Context) {
+			// system.clusters lists every Replicated database on the server, including ones created by
+			// users with their own shard and replica names, next to the static clusters from
+			// remote_servers whose names are empty. None of those parse as replica numbers.
+			id0 := v1.ClickHouseReplicaID{ShardID: 0, Index: 0}
+			conn, err := cmd.getConn(id0)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(conn.Exec(ctx, "CREATE DATABASE named ENGINE = Replicated('/clickhouse/databases/named', 'shard-a', 'replica-b')")).To(Succeed())
+
+			DeferCleanup(func(ctx context.Context) {
+				Expect(conn.Exec(ctx, "DROP DATABASE named SYNC")).To(Succeed())
+			})
+
+			Expect(cmd.CleanupDatabaseReplicas(ctx, cmd.log, map[v1.ClickHouseReplicaID]struct{}{})).To(Succeed())
+		})
+
 		It("should do nothing if still active", func(ctx context.Context) {
 			*cmd.cluster.Spec.Replicas = testReplicas - 1
 			err := cmd.CleanupDatabaseReplicas(ctx, cmd.log, map[v1.ClickHouseReplicaID]struct{}{})

From e0d3f8e70dbfef6712704f6c5885ffaf992a055d Mon Sep 17 00:00:00 2001
From: Vasiliy Koshkin <61789920+melancholictheory@users.noreply.github.com>
Date: Tue, 29 Sep 2026 20:39:50 +0300
Subject: [PATCH 2/2] ci: re-run after an unrelated operator-upgrade e2e
 failure