Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Col delete func #3586

Merged
merged 12 commits into from
May 20, 2024
22 changes: 18 additions & 4 deletions db/columns/operations/drop.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
"""The function in this module wraps SQL functions that drop columns."""
"""The functions in this module wrap SQL functions that drop columns."""
from db import connection as db_conn


def drop_column(table_oid, column_attnum, engine):
"""
Drop the given columns from the given table.
Drop the given column from the given table.

Args:
table_oid: OID of the table whose columns we'll drop.
column_attnum: The attnums of the columns to drop.
table_oid: OID of the table whose column we'll drop.
column_attnum: The attnum of the column to drop.
engine: SQLAlchemy engine object for connecting.

Returns:
Expand All @@ -17,3 +17,17 @@ def drop_column(table_oid, column_attnum, engine):
return db_conn.execute_msar_func_with_engine(
engine, 'drop_columns', table_oid, column_attnum
).fetchone()[0]


def drop_columns_from_table(table_oid, column_attnums, conn):
"""
Drop the given columns from the given table.

Args:
table_oid: OID of the table whose columns we'll drop.
column_attnums: The attnums of the columns to drop.
conn: A psycopg connection to the relevant database.
"""
return db_conn.exec_msar_func(
conn, 'drop_columns', table_oid, *column_attnums
).fetchone()[0]
25 changes: 23 additions & 2 deletions db/sql/0_msar.sql → db/sql/00_msar.sql
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,26 @@ END;
$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;


CREATE OR REPLACE FUNCTION
msar.get_relation_name_or_null(rel_id oid) RETURNS text AS $$/*
Return the name for a given relation (e.g., table), qualified or quoted as appropriate.

In cases where the relation is already included in the search path, the returned name will not be
fully-qualified.

The relation *must* be in the pg_class table to use this function. This function will return NULL if
no corresponding relation can be found.

Args:
rel_id: The OID of the relation.
*/
SELECT CASE
WHEN EXISTS (SELECT oid FROM pg_class WHERE oid=rel_id) THEN rel_id::regclass::text
END
$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;



DROP FUNCTION IF EXISTS msar.get_relation_oid(text, text) CASCADE;
CREATE OR REPLACE FUNCTION
msar.get_relation_oid(sch_name text, rel_name text) RETURNS oid AS $$/*
Expand Down Expand Up @@ -1117,9 +1137,10 @@ DECLARE col_names text[];
BEGIN
SELECT array_agg(quote_ident(attname))
FROM pg_attribute
WHERE attrelid=tab_id AND ARRAY[attnum::integer] <@ col_ids
WHERE attrelid=tab_id AND NOT attisdropped AND ARRAY[attnum::integer] <@ col_ids
INTO col_names;
RETURN __msar.drop_columns(__msar.get_relation_name(tab_id), variadic col_names);
PERFORM __msar.drop_columns(msar.get_relation_name_or_null(tab_id), variadic col_names);
RETURN array_length(col_names, 1);
END;
$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;

Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion db/sql/2_msar_views.sql → db/sql/20_msar_views.sql
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
--
-- This file creates a schema `msar_views` where internal mathesar views will be stored.
--
-- For naming conventions, see 0_msar.sql
-- For naming conventions, see 00_msar.sql
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
CREATE SCHEMA IF NOT EXISTS msar_views;
Expand Down
4 changes: 2 additions & 2 deletions db/sql/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
from db.connection import load_file_with_engine

FILE_DIR = os.path.abspath(os.path.dirname(__file__))
MSAR_SQL = os.path.join(FILE_DIR, '0_msar.sql')
MSAR_AGGREGATE_SQL = os.path.join(FILE_DIR, '3_msar_custom_aggregates.sql')
MSAR_SQL = os.path.join(FILE_DIR, '00_msar.sql')
MSAR_AGGREGATE_SQL = os.path.join(FILE_DIR, '30_msar_custom_aggregates.sql')


def install(engine):
Expand Down
11 changes: 11 additions & 0 deletions db/sql/test_0_msar.sql → db/sql/test_00_msar.sql
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ END;
$$ LANGUAGE plpgsql;


CREATE OR REPLACE FUNCTION test_drop_columns_ne_oid() RETURNS SETOF TEXT AS $$
BEGIN
CREATE TABLE "12345" (bleh text, bleh2 numeric);
PERFORM msar.drop_columns(12345, 1);
RETURN NEXT has_column(
'12345', 'bleh', 'Doesn''t drop columns of stupidly-named table'
);
END;
$$ LANGUAGE plpgsql;


CREATE OR REPLACE FUNCTION test_drop_columns_names() RETURNS SETOF TEXT AS $$
BEGIN
PERFORM __setup_drop_columns();
Expand Down
4 changes: 2 additions & 2 deletions db/sql/test_startup.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ $$ LANGUAGE plpgsql;
CALL raise_notice('Creating testing DB');
CREATE DATABASE mathesar_testing;
\c mathesar_testing
\ir 0_msar.sql
\ir test_0_msar.sql
\ir 00_msar.sql
\ir test_00_msar.sql
18 changes: 18 additions & 0 deletions db/tests/columns/operations/test_drop.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from unittest.mock import patch
from db.columns.operations import drop as col_drop


def test_drop_columns():
with patch.object(col_drop.db_conn, 'exec_msar_func') as mock_exec:
mock_exec.return_value.fetchone = lambda: (3,)
result = col_drop.drop_columns_from_table(123, [1, 3, 5], 'conn')
mock_exec.assert_called_once_with('conn', 'drop_columns', 123, 1, 3, 5)
assert result == 3


def test_get_column_info_for_table():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function name doesn't make sense.

with patch.object(col_drop.db_conn, 'exec_msar_func') as mock_exec:
mock_exec.return_value.fetchone = lambda: (1,)
result = col_drop.drop_columns_from_table(123, [1], 'conn')
mock_exec.assert_called_once_with('conn', 'drop_columns', 123, 1)
assert result == 1
1 change: 1 addition & 0 deletions docs/docs/api/rpc.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ To use an RPC function:
options:
members:
- list_
- delete
- ColumnListReturn
- ColumnInfo
- TypeOptions
Expand Down
23 changes: 23 additions & 0 deletions mathesar/rpc/columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from modernrpc.auth.basic import http_basic_auth_login_required

from db.columns.operations.select import get_column_info_for_table
from db.columns.operations.drop import drop_columns_from_table
from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions
from mathesar.rpc.utils import connect
from mathesar.utils.columns import get_raw_display_options
Expand Down Expand Up @@ -155,3 +156,25 @@ def list_(*, table_oid: int, database_id: int, **kwargs) -> ColumnListReturn:
column_info=column_info,
display_options=display_options,
)


@rpc_method(name="columns.delete")
@http_basic_auth_login_required
@handle_rpc_exceptions
def delete(
*, column_attnums: list[int], table_oid: int, database_id: int, **kwargs
) -> int:
"""
Delete columns from a table.

Args:
column_attnums: A list of attnums of columns to delete.
table_oid: Identity of the table in the user's database.
database_id: The Django id of the database containing the table.

Returns:
The number of columns dropped.
"""
user = kwargs.get(REQUEST_KEY).user
with connect(database_id, user) as conn:
return drop_columns_from_table(table_oid, column_attnums, conn)
33 changes: 33 additions & 0 deletions mathesar/tests/rpc/test_columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,36 @@ def mock_display_options(_database_id, _table_oid, attnums, user):
}
actual_col_list = columns.list_(table_oid=23457, database_id=2, request=request)
assert actual_col_list == expect_col_list


def test_columns_delete(rf, monkeypatch):
request = rf.post('/api/rpc/v0/', data={})
request.user = User(username='alice', password='pass1234')
table_oid = 23457
database_id = 2
column_attnums = [2, 3, 8]

@contextmanager
def mock_connect(_database_id, user):
if _database_id == 2 and user.username == 'alice':
try:
yield True
finally:
pass
else:
raise AssertionError('incorrect parameters passed')

def mock_column_drop(_table_oid, _column_attnums, conn):
if _table_oid != table_oid or _column_attnums != column_attnums:
raise AssertionError('incorrect parameters passed')
return 3

monkeypatch.setattr(columns, 'connect', mock_connect)
monkeypatch.setattr(columns, 'drop_columns_from_table', mock_column_drop)
actual_result = columns.delete(
column_attnums=column_attnums,
table_oid=table_oid,
database_id=database_id,
request=request
)
assert actual_result == 3
5 changes: 5 additions & 0 deletions mathesar/tests/rpc/test_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@


METHODS = [
(
columns.delete,
"columns.delete",
[user_is_authenticated]
),
(
columns.list_,
"columns.list",
Expand Down
Loading