feat(backends): fixup drop_database() when passing a specific catalog (#11110)

Co-authored-by: Phillip Cloud <[email protected]>

Author: NickCrews

ibis/backends/clickhouse/__init__.py

@@ -570,8 +570,12 @@ def create_database(
         with self._safe_raw_sql(src):
             pass
 
-    def drop_database(self, name: str, /, *, force: bool = False) -> None:
-        src = sge.Drop(this=sg.to_identifier(name), kind="DATABASE", exists=force)
+    def drop_database(
+        self, name: str, /, *, catalog: str | None = None, force: bool = False
+    ) -> None:
+        src = sge.Drop(
+            this=sg.table(name, catalog=catalog), kind="DATABASE", exists=force
+        )
         with self._safe_raw_sql(src):
             pass
 

ibis/backends/duckdb/__init__.py

@@ -508,7 +508,7 @@ def create_database(
     def drop_database(
         self, name: str, /, *, catalog: str | None = None, force: bool = False
     ) -> None:
-        if catalog is not None:
+        if catalog is not None and catalog != self.current_catalog:
             raise exc.UnsupportedOperationError(
                 "DuckDB cannot drop a database in another catalog."
             )

ibis/backends/duckdb/tests/test_catalog.py

@@ -1,27 +1,31 @@
 from __future__ import annotations
 
+import duckdb
 import pandas as pd
 import pandas.testing as tm
 import pytest
 
 import ibis
 import ibis.common.exceptions as exc
+from ibis.util import gen_name
 
 
-@pytest.fixture(scope="session")
-def external_duckdb_file(tmpdir_factory):  # pragma: no cover
-    ddb_path = str(tmpdir_factory.mktemp("data") / "starwars.ddb")
+@pytest.fixture
+def external_duckdb_file(tmpdir):  # pragma: no cover
+    ddb_path = str(tmpdir / "starwars.ddb")
     con = ibis.duckdb.connect(ddb_path)
 
-    starwars_df = pd.DataFrame(
-        {
-            "name": ["Luke Skywalker", "C-3PO", "R2-D2"],
-            "height": [172, 167, 96],
-            "mass": [77.0, 75.0, 32.0],
-        }
-    )
-    con.create_table("starwars", obj=starwars_df)
-    con.disconnect()
+    try:
+        starwars_df = pd.DataFrame(
+            {
+                "name": ["Luke Skywalker", "C-3PO", "R2-D2"],
+                "height": [172, 167, 96],
+                "mass": [77.0, 75.0, 32.0],
+            }
+        )
+        con.create_table("starwars", obj=starwars_df)
+    finally:
+        con.disconnect()
 
     return ddb_path, starwars_df
 
@@ -30,44 +34,57 @@ def test_read_write_external_catalog(con, external_duckdb_file, monkeypatch):
     monkeypatch.setattr(ibis.options, "default_backend", con)
 
     ddb_path, starwars_df = external_duckdb_file
-    con.attach(ddb_path, name="ext")
+    name = gen_name("ext")
+    con.attach(ddb_path, name=name)
 
     # Read from catalog
-    assert "ext" in con.list_catalogs()
-    assert "main" in con.list_databases(catalog="ext")
+    assert name in con.list_catalogs()
+    assert "main" in con.list_databases(catalog=name)
 
-    assert "starwars" in con.list_tables(database="ext.main")
+    db = f"{name}.main"
+
+    assert "starwars" in con.list_tables(database=db)
     assert "starwars" not in con.list_tables()
 
-    starwars = con.table("starwars", database="ext.main")
+    starwars = con.table("starwars", database=db)
     tm.assert_frame_equal(starwars.to_pandas(), starwars_df)
 
     # Write to catalog
     t = ibis.memtable([{"a": 1, "b": "foo"}, {"a": 2, "b": "baz"}])
 
-    _ = con.create_table("t2", obj=t, database="ext.main")
+    _ = con.create_table("t2", obj=t, database=db)
 
-    assert "t2" in con.list_tables(database="ext.main")
+    assert "t2" in con.list_tables(database=db)
     assert "t2" not in con.list_tables()
 
-    table = con.table("t2", database="ext.main")
+    table = con.table("t2", database=db)
 
     tm.assert_frame_equal(t.to_pandas(), table.to_pandas())
 
     # Overwrite table in catalog
 
     t_overwrite = ibis.memtable([{"a": 8, "b": "bing"}, {"a": 9, "b": "bong"}])
 
-    _ = con.create_table("t2", obj=t_overwrite, database="ext.main", overwrite=True)
+    _ = con.create_table("t2", obj=t_overwrite, database=db, overwrite=True)
 
-    assert "t2" in con.list_tables(database="ext.main")
+    assert "t2" in con.list_tables(database=db)
     assert "t2" not in con.list_tables()
 
-    table = con.table("t2", database="ext.main")
+    table = con.table("t2", database=db)
 
     tm.assert_frame_equal(t_overwrite.to_pandas(), table.to_pandas())
 
 
 def test_raise_if_catalog_and_temp(con):
     with pytest.raises(exc.UnsupportedArgumentError):
         con.create_table("some_table", obj="hi", temp=True, database="ext.main")
+
+
+def test_cant_drop_database_external_catalog(con, tmpdir):
+    name = gen_name("foobar")
+    path = str(tmpdir / "f{name}.ddb")
+    with duckdb.connect(path):
+        pass
+    con.attach(path)
+    with pytest.raises(exc.UnsupportedOperationError):
+        con.drop_database("main", catalog=name)

ibis/backends/mysql/__init__.py

@@ -255,10 +255,12 @@ def create_database(self, name: str, force: bool = False) -> None:
         with self.begin() as cur:
             cur.execute(sql)
 
-    def drop_database(self, name: str, force: bool = False) -> None:
-        sql = sge.Drop(kind="DATABASE", exists=force, this=sg.to_identifier(name)).sql(
-            self.name
-        )
+    def drop_database(
+        self, name: str, *, catalog: str | None = None, force: bool = False
+    ) -> None:
+        sql = sge.Drop(
+            kind="DATABASE", exists=force, this=sg.table(name, catalog=catalog)
+        ).sql(self.name)
         with self.begin() as cur:
             cur.execute(sql)
 

ibis/backends/postgres/__init__.py

@@ -600,7 +600,7 @@ def drop_database(
 
         sql = sge.Drop(
             kind="SCHEMA",
-            this=sg.table(name, catalog=catalog),
+            this=sg.table(name),
             exists=force,
             cascade=cascade,
         ).sql(self.dialect)

ibis/backends/risingwave/__init__.py

@@ -401,7 +401,7 @@ def drop_database(
 
         sql = sge.Drop(
             kind="SCHEMA",
-            this=sg.table(name, catalog=catalog),
+            this=sg.table(name),
             exists=force,
             cascade=cascade,
         )

ibis/backends/tests/test_client.py

@@ -1441,11 +1441,16 @@ def test_create_catalog(con_create_catalog):
     assert catalog not in con_create_catalog.list_catalogs()
 
 
-def test_create_database(con_create_database):
+@pytest.mark.parametrize("catalog", [None, "current_catalog"])
+def test_create_database(con_create_database, catalog):
     database = gen_name("test_create_database")
     con_create_database.create_database(database)
     assert database in con_create_database.list_databases()
-    con_create_database.drop_database(database)
+    if catalog is None:
+        catalog = None
+    else:
+        catalog = getattr(con_create_database, "current_catalog", None)
+    con_create_database.drop_database(database, catalog=catalog)
     assert database not in con_create_database.list_databases()