From aa8c4bea8f5aa18ccd94fefaa5ce17daf0ff1464 Mon Sep 17 00:00:00 2001 From: Dhruv Gupta Date: Mon, 9 Nov 2026 06:27:39 -0500 Subject: [PATCH 1/2] fix(snowflake): compare the bare database name when dropping a catalog `_drop_catalog` built the `DATABASE_NAME` literal for the existence check from the identifier object itself, so a catalog name that has to be quoted (for example `my-db__dev`) was compared as `'"my-db__dev"'`, including the double quotes. The check never matched, so expired environment catalogs with such names were never dropped and the janitor warned on every run that the database did not look managed. Use the identifier's name instead, which is what `CREATE DATABASE` stored. Signed-off-by: Dhruv Gupta --- sqlmesh/core/engine_adapter/snowflake.py | 2 +- tests/core/engine_adapter/test_snowflake.py | 24 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/sqlmesh/core/engine_adapter/snowflake.py b/sqlmesh/core/engine_adapter/snowflake.py index 758e4847c1..dace876bc7 100644 --- a/sqlmesh/core/engine_adapter/snowflake.py +++ b/sqlmesh/core/engine_adapter/snowflake.py @@ -174,7 +174,7 @@ def _drop_catalog(self, catalog_name: exp.Identifier) -> None: .from_(exp.to_table("information_schema.databases")) .where( exp.and_( - exp.column("database_name").eq(exp.Literal.string(catalog_name)), + exp.column("database_name").eq(exp.Literal.string(catalog_name.name)), exp.column("comment").eq(exp.Literal.string(c.SQLMESH_MANAGED)), ) ) diff --git a/tests/core/engine_adapter/test_snowflake.py b/tests/core/engine_adapter/test_snowflake.py index 89f19fc588..ba416a84d7 100644 --- a/tests/core/engine_adapter/test_snowflake.py +++ b/tests/core/engine_adapter/test_snowflake.py @@ -1187,3 +1187,27 @@ def test_drop_catalog(snowflake_mocked_engine_adapter: SnowflakeEngineAdapter) - """SELECT 1 FROM "INFORMATION_SCHEMA"."DATABASES" WHERE "DATABASE_NAME" = 'foo' AND "COMMENT" = 'sqlmesh_managed'""", 'DROP DATABASE IF EXISTS "foo"', ] + + +@pytest.mark.parametrize( + "catalog_name, expected_name", + [ + # names that need quoting are parsed into quoted identifiers + ("my-db__dev", "my-db__dev"), + (exp.to_identifier("my db", quoted=True), "my db"), + (exp.to_identifier("MyDb", quoted=True), "MyDb"), + ], +) +def test_drop_catalog_quoted_name( + snowflake_mocked_engine_adapter: SnowflakeEngineAdapter, + catalog_name: t.Union[str, exp.Identifier], + expected_name: str, +) -> None: + adapter = snowflake_mocked_engine_adapter + adapter.drop_catalog(catalog_name) + + # the existence check must compare against the bare database name, without identifier quotes + assert to_sql_calls(adapter) == [ + f"""SELECT 1 FROM "INFORMATION_SCHEMA"."DATABASES" WHERE "DATABASE_NAME" = '{expected_name}' AND "COMMENT" = 'sqlmesh_managed'""", + f'DROP DATABASE IF EXISTS "{expected_name}"', + ] From 9840f440dae10ae9f59b118449644d58d348b383 Mon Sep 17 00:00:00 2001 From: Dhruv Gupta Date: Fri, 13 Nov 2026 16:02:04 -0500 Subject: [PATCH 2/2] test(snowflake): cover both drop_catalog branches for a quoted name The quoted-name unit test now sets the existence check result: a row gives the DROP, and no row gives only the SELECT. The Snowflake integration test creates a hyphenated catalog, checks that it is stored under the bare name, drops it, and confirms it is gone. Signed-off-by: Dhruv Gupta --- .../integration/test_integration_snowflake.py | 27 +++++++++++++++++++ tests/core/engine_adapter/test_snowflake.py | 13 ++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/tests/core/engine_adapter/integration/test_integration_snowflake.py b/tests/core/engine_adapter/integration/test_integration_snowflake.py index 7f3c38be46..c9ce95654c 100644 --- a/tests/core/engine_adapter/integration/test_integration_snowflake.py +++ b/tests/core/engine_adapter/integration/test_integration_snowflake.py @@ -315,6 +315,33 @@ def fetch_database_names() -> t.Set[str]: assert fetch_database_names() == {non_sqlmesh_managed_catalog} +def test_create_drop_catalog_quoted_name(ctx: TestContext, engine_adapter: SnowflakeEngineAdapter): + # a name with a hyphen has to be quoted, so it only matches the existence check in + # drop_catalog if the check compares against the bare name + sqlmesh_managed_catalog = ctx.add_test_suffix("env-dev") + + initial_catalog = engine_adapter.get_current_catalog() + assert initial_catalog + + ctx._catalogs.append(sqlmesh_managed_catalog) # so it still gets cleaned up if the test fails + engine_adapter.create_catalog(sqlmesh_managed_catalog) + + def fetch_database_names() -> t.Set[str]: + engine_adapter.set_current_catalog(initial_catalog) + return { + str(r[0]) + for r in engine_adapter.fetchall( + f"select database_name from information_schema.databases where database_name like '%{ctx.test_id}'" + ) + } + + # the database is stored under the bare name, without identifier quotes + assert fetch_database_names() == {sqlmesh_managed_catalog} + + engine_adapter.drop_catalog(sqlmesh_managed_catalog) + assert fetch_database_names() == set() + + def test_rows_tracker( ctx: TestContext, engine_adapter: SnowflakeEngineAdapter, mocker: MockerFixture ): diff --git a/tests/core/engine_adapter/test_snowflake.py b/tests/core/engine_adapter/test_snowflake.py index 8c93b160e4..384d6bae5e 100644 --- a/tests/core/engine_adapter/test_snowflake.py +++ b/tests/core/engine_adapter/test_snowflake.py @@ -1210,14 +1210,25 @@ def test_drop_catalog_quoted_name( expected_name: str, ) -> None: adapter = snowflake_mocked_engine_adapter + exists_check = f"""SELECT 1 FROM "INFORMATION_SCHEMA"."DATABASES" WHERE "DATABASE_NAME" = '{expected_name}' AND "COMMENT" = 'sqlmesh_managed'""" + + # the existence check finds the sqlmesh_managed database, so it is dropped + adapter.cursor.fetchone.return_value = (1,) adapter.drop_catalog(catalog_name) # the existence check must compare against the bare database name, without identifier quotes assert to_sql_calls(adapter) == [ - f"""SELECT 1 FROM "INFORMATION_SCHEMA"."DATABASES" WHERE "DATABASE_NAME" = '{expected_name}' AND "COMMENT" = 'sqlmesh_managed'""", + exists_check, f'DROP DATABASE IF EXISTS "{expected_name}"', ] + # the existence check finds nothing, so the database is left alone + adapter.cursor.execute.reset_mock() + adapter.cursor.fetchone.return_value = None + adapter.drop_catalog(catalog_name) + + assert to_sql_calls(adapter) == [exists_check] + def _make_snowflake_clustering_adapter( make_mocked_engine_adapter: t.Callable,