Merge pull request #3608 from mathesar-foundation/table_delete

Implement `tables.delete` rpc endpoint

db/sql/00_msar.sql

@@ -2135,16 +2135,21 @@ $$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;
 
 
 CREATE OR REPLACE FUNCTION
-msar.drop_table(tab_id oid, cascade_ boolean, if_exists boolean) RETURNS text AS $$/*
-Drop a table, returning the command executed.
+msar.drop_table(tab_id oid, cascade_ boolean) RETURNS text AS $$/*
+Drop a table, returning the fully qualified name of the dropped table.
 
 Args:
   tab_id: The OID of the table to drop
   cascade_: Whether to drop dependent objects.
-  if_exists_: Whether to ignore an error if the table doesn't exist
 */
+DECLARE relation_name text;
 BEGIN
-  RETURN __msar.drop_table(__msar.get_relation_name(tab_id), cascade_, if_exists);
+  relation_name := msar.get_relation_name_or_null(tab_id);
+  -- if_exists doesn't work while working with oids because
+  -- the SQL query gets parameterized with tab_id instead of relation_name
+  -- since we're unable to find the relation_name for a non existing table. 
+  PERFORM __msar.drop_table(relation_name, cascade_, if_exists => false);
+  RETURN relation_name;
 END;
 $$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;
 

db/sql/test_00_msar.sql

@@ -72,24 +72,12 @@ DECLARE
 BEGIN
   PERFORM __setup_drop_tables();
   rel_id := 'dropme'::regclass::oid;
-  PERFORM msar.drop_table(tab_id => rel_id, cascade_ => false, if_exists => false);
+  PERFORM msar.drop_table(tab_id => rel_id, cascade_ => false);
   RETURN NEXT hasnt_table('dropme', 'Drops table');
 END;
 $$ LANGUAGE plpgsql;
 
 
-CREATE OR REPLACE FUNCTION test_drop_table_oid_if_exists() RETURNS SETOF TEXT AS $$
-DECLARE
-  rel_id oid;
-BEGIN
-  PERFORM __setup_drop_tables();
-  rel_id := 'dropme'::regclass::oid;
-  PERFORM msar.drop_table(tab_id => rel_id, cascade_ => false, if_exists => true);
-  RETURN NEXT hasnt_table('dropme', 'Drops table with IF EXISTS');
-END;
-$$ LANGUAGE plpgsql;
-
-
 CREATE OR REPLACE FUNCTION test_drop_table_oid_restricted_fkey() RETURNS SETOF TEXT AS $$
 DECLARE
   rel_id oid;
@@ -99,7 +87,7 @@ BEGIN
   CREATE TABLE
     dependent (id SERIAL PRIMARY KEY, col1 integer REFERENCES dropme);
   RETURN NEXT throws_ok(
-    format('SELECT msar.drop_table(tab_id => %s, cascade_ => false, if_exists => true);', rel_id),
+    format('SELECT msar.drop_table(tab_id => %s, cascade_ => false);', rel_id),
     '2BP01',
     'cannot drop table dropme because other objects depend on it',
     'Table dropper throws for dependent objects'
@@ -116,7 +104,7 @@ BEGIN
   rel_id := 'dropme'::regclass::oid;
   CREATE TABLE
     dependent (id SERIAL PRIMARY KEY, col1 integer REFERENCES dropme);
-  PERFORM msar.drop_table(tab_id => rel_id, cascade_ => true, if_exists => false);
+  PERFORM msar.drop_table(tab_id => rel_id, cascade_ => true);
   RETURN NEXT hasnt_table('dropme', 'Drops table with dependent using CASCADE');
 END;
 $$ LANGUAGE plpgsql;

db/tables/operations/drop.py

@@ -1,5 +1,21 @@
-from db.connection import execute_msar_func_with_engine
+from db.connection import execute_msar_func_with_engine, exec_msar_func
 
 
 def drop_table(name, schema, engine, cascade=False, if_exists=False):
     execute_msar_func_with_engine(engine, 'drop_table', schema, name, cascade, if_exists)
+
+
+def drop_table_from_database(table_oid, conn, cascade=False):
+    """
+    Drop a table.
+
+    Args:
+        table_oid: OID of the table to drop.
+        cascade: Whether to drop the dependent objects.
+
+    Returns:
+        Returns the fully qualified name of the dropped table.
+    """
+    return exec_msar_func(
+        conn, 'drop_table', table_oid, cascade
+    ).fetchone()[0]

docs/docs/api/rpc.md

@@ -60,6 +60,7 @@ To use an RPC function:
     options:
       members:
       - list_
+      - delete
       - TableInfo
 
 ---

mathesar/rpc/tables.py

@@ -4,6 +4,7 @@
 from modernrpc.auth.basic import http_basic_auth_login_required
 
 from db.tables.operations.select import get_table_info
+from db.tables.operations.drop import drop_table_from_database
 from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions
 from mathesar.rpc.utils import connect
 
@@ -44,3 +45,25 @@ def list_(*, schema_oid: int, database_id: int, **kwargs) -> list[TableInfo]:
     return [
         TableInfo(tab) for tab in raw_table_info
     ]
+
+
+@rpc_method(name="tables.delete")
+@http_basic_auth_login_required
+@handle_rpc_exceptions
+def delete(
+    *, table_oid: int, database_id: int, cascade: bool = False, **kwargs
+) -> str:
+    """
+    Delete a table from a schema.
+
+    Args:
+        table_oid: Identity of the table in the user's database.
+        database_id: The Django id of the database containing the table.
+        cascade: Whether to drop the dependent objects.
+
+    Returns:
+        The name of the dropped table.
+    """
+    user = kwargs.get(REQUEST_KEY).user
+    with connect(database_id, user) as conn:
+        return drop_table_from_database(table_oid, conn, cascade)

mathesar/tests/rpc/test_endpoints.py

@@ -44,6 +44,11 @@
         "tables.list",
         [user_is_authenticated]
     ),
+    (
+        tables.delete,
+        "tables.delete",
+        [user_is_authenticated]
+    )
 ]
 
 

mathesar/tests/rpc/test_tables.py

@@ -62,3 +62,30 @@ def mock_table_info(_schema_oid, conn):
     ]
     actual_table_list = tables.list_(schema_oid=2200, database_id=11, request=request)
     assert actual_table_list == expect_table_list
+
+
+def test_tables_delete(rf, monkeypatch):
+    request = rf.post('/api/rpc/v0', data={})
+    request.user = User(username='alice', password='pass1234')
+    table_oid = 1964474
+    database_id = 11
+
+    @contextmanager
+    def mock_connect(_database_id, user):
+        if _database_id == database_id and user.username == 'alice':
+            try:
+                yield True
+            finally:
+                pass
+        else:
+            raise AssertionError('incorrect parameters passed')
+
+    def mock_drop_table(_table_oid, conn, cascade):
+        if _table_oid != table_oid:
+            raise AssertionError('incorrect parameters passed')
+        return 'public."Table 0"'
+
+    monkeypatch.setattr(tables, 'connect', mock_connect)
+    monkeypatch.setattr(tables, 'drop_table_from_database', mock_drop_table)
+    deleted_table = tables.delete(table_oid=1964474, database_id=11, request=request)
+    assert deleted_table == 'public."Table 0"'