Merge pull request #3616 from mathesar-foundation/columns_add

Add `columns.add` RPC function

db/columns/operations/create.py

@@ -5,39 +5,18 @@
 from alembic.operations import Operations
 from psycopg.errors import InvalidTextRepresentation, InvalidParameterValue
 
-from db.columns.defaults import DEFAULT, NAME, NULLABLE, TYPE, DESCRIPTION
+from db import connection as db_conn
+from db.columns.defaults import DEFAULT, NAME, NULLABLE, DESCRIPTION
 from db.columns.exceptions import InvalidDefaultError, InvalidTypeOptionError
-from db.connection import execute_msar_func_with_engine
 from db.tables.operations.select import reflect_table_from_oid
 from db.types.base import PostgresType
 from db.metadata import get_empty_metadata
 
 
 def create_column(engine, table_oid, column_data):
-    column_name = (column_data.get(NAME) or '').strip() or None
-    column_type_id = (
-        column_data.get(
-            # TYPE = 'sa_type'. This is coming straight from the API.
-            # TODO Determine whether we actually need 'sa_type' and 'type'
-            TYPE, column_data.get("type")
-        )
-        or PostgresType.CHARACTER_VARYING.id
-    )
-    column_type_options = column_data.get("type_options", {})
-    column_nullable = column_data.get(NULLABLE, True)
-    default_value = column_data.get(DEFAULT, {}).get('value')
-    column_description = column_data.get(DESCRIPTION)
-    col_create_def = [
-        {
-            "name": column_name,
-            "type": {"name": column_type_id, "options": column_type_options},
-            "not_null": not column_nullable,
-            "default": default_value,
-            "description": column_description,
-        }
-    ]
+    col_create_def = [_transform_column_create_dict(column_data)]
     try:
-        curr = execute_msar_func_with_engine(
+        curr = db_conn.execute_msar_func_with_engine(
             engine, 'add_columns',
             table_oid,
             json.dumps(col_create_def)
@@ -49,6 +28,64 @@ def create_column(engine, table_oid, column_data):
     return curr.fetchone()[0]
 
 
+def add_columns_to_table(table_oid, column_data_list, conn):
+    """
+    Add columns to the given table.
+
+    For a description of the members of column_data_list, see
+    _transform_column_create_dict
+
+    Args:
+        table_oid: The OID of the table whose columns we'll alter.
+        column_data_list: A list of dicts describing columns to add.
+        conn: A psycopg connection.
+    """
+    transformed_column_data = [
+        _transform_column_create_dict(col) for col in column_data_list
+    ]
+    result = db_conn.exec_msar_func(
+        conn, 'add_columns', table_oid, json.dumps(transformed_column_data)
+    ).fetchone()[0]
+    return result
+
+
+# TODO This function wouldn't be needed if we had the same form in the DB
+# as the RPC API function.
+def _transform_column_create_dict(data):
+    """
+    Transform the data dict into the form needed for the DB functions.
+
+    Input data form:
+    {
+        "name": <str>,
+        "type": <str>,
+        "type_options": <dict>,
+        "nullable": <bool>,
+        "default": {"value": <any>}
+        "description": <str>
+    }
+
+    Output form:
+    {
+        "type": {"name": <str>, "options": <dict>},
+        "name": <str>,
+        "not_null": <bool>,
+        "default": <any>,
+        "description": <str>
+    }
+    """
+    return {
+        "name": (data.get(NAME) or '').strip() or None,
+        "type": {
+            "name": data.get("type") or PostgresType.CHARACTER_VARYING.id,
+            "options": data.get("type_options", {})
+        },
+        "not_null": not data.get(NULLABLE, True),
+        "default": data.get(DEFAULT, {}).get('value'),
+        "description": data.get(DESCRIPTION),
+    }
+
+
 def bulk_create_mathesar_column(engine, table_oid, columns, schema):
     # TODO reuse metadata
     table = reflect_table_from_oid(table_oid, engine, metadata=get_empty_metadata())
@@ -67,7 +104,7 @@ def duplicate_column(
         copy_data=True,
         copy_constraints=True
 ):
-    curr = execute_msar_func_with_engine(
+    curr = db_conn.execute_msar_func_with_engine(
         engine,
         'copy_column',
         table_oid,

db/tests/columns/operations/test_create.py

@@ -21,38 +21,36 @@ def test_type_list_completeness(engine):
 
 
 @pytest.mark.parametrize(
-    "in_name,out_name", [('test1', 'test1'), ('', None), (None, None)]
+    "in_name,out_name", [("test1", "test1"), ("", None), (None, None)]
 )
-def test_create_column_name(engine_with_schema, in_name, out_name):
+def test_add_columns_name(in_name, out_name):
     """
     Here, we just check that the PostgreSQL function is called properly, when
     given a (maybe empty) name param
     """
-    engine, schema = engine_with_schema
-    with patch.object(col_create, "execute_msar_func_with_engine") as mock_exec:
-        col_create.create_column(engine, 12345, {"name": in_name})
+    with patch.object(col_create.db_conn, "exec_msar_func") as mock_exec:
+        col_create.add_columns_to_table(123, [{"name": in_name}], "conn")
     call_args = mock_exec.call_args_list[0][0]
-    assert call_args[0] == engine
+    assert call_args[0] == "conn"
     assert call_args[1] == "add_columns"
-    assert call_args[2] == 12345
+    assert call_args[2] == 123
     assert json.loads(call_args[3])[0]["name"] == out_name
 
 
 @pytest.mark.parametrize(
     "in_type,out_type", [("numeric", "numeric"), (None, "character varying")]
 )
-def test_create_column_type(engine_with_schema, in_type, out_type):
+def test_add_columns_type(in_type, out_type):
     """
     Here, we just check that the PostgreSQL function is called properly when
     given a (maybe empty) type
     """
-    engine, schema = engine_with_schema
-    with patch.object(col_create, "execute_msar_func_with_engine") as mock_exec:
-        col_create.create_column(engine, 12345, {"type": in_type})
+    with patch.object(col_create.db_conn, "exec_msar_func") as mock_exec:
+        col_create.add_columns_to_table(123, [{"type": in_type}], "conn")
     call_args = mock_exec.call_args_list[0][0]
-    assert call_args[0] == engine
+    assert call_args[0] == "conn"
     assert call_args[1] == "add_columns"
-    assert call_args[2] == 12345
+    assert call_args[2] == 123
     actual_col_data = json.loads(call_args[3])[0]
     assert actual_col_data["name"] is None
     assert actual_col_data["type"]["name"] == out_type
@@ -62,26 +60,25 @@ def test_create_column_type(engine_with_schema, in_type, out_type):
 @pytest.mark.parametrize(
     "in_options,out_options", [({"foo": "bar"}, {"foo": "bar"}), (None, None), ({}, {})]
 )
-def test_create_column_type_options(engine_with_schema, in_options, out_options):
+def test_add_columns_type_options(in_options, out_options):
     """
     Here, we just check that the PostgreSQL function is called properly when
     given a (maybe empty) type options dict.
     """
-    engine, schema = engine_with_schema
-    with patch.object(col_create, "execute_msar_func_with_engine") as mock_exec:
-        col_create.create_column(engine, 12345, {"type_options": in_options})
+    with patch.object(col_create.db_conn, "exec_msar_func") as mock_exec:
+        col_create.add_columns_to_table(123, [{"type_options": in_options}], "conn")
     call_args = mock_exec.call_args_list[0][0]
-    assert call_args[0] == engine
+    assert call_args[0] == "conn"
     assert call_args[1] == "add_columns"
-    assert call_args[2] == 12345
+    assert call_args[2] == 123
     assert json.loads(call_args[3])[0]["type"]["name"] == "character varying"
     assert json.loads(call_args[3])[0]["type"]["options"] == out_options
 
 
 def test_duplicate_column_smoke(engine_with_schema):
     """This is just a smoke test, since the underlying function is trivial."""
     engine, schema = engine_with_schema
-    with patch.object(col_create, "execute_msar_func_with_engine") as mock_exec:
+    with patch.object(col_create.db_conn, "execute_msar_func_with_engine") as mock_exec:
         col_create.duplicate_column(
             12345,
             4,

docs/docs/api/rpc.md

@@ -72,6 +72,7 @@ To use an RPC function:
     options:
       members:
       - list_
+      - add
       - patch
       - delete
       - ColumnListReturn

mathesar/rpc/columns.py

@@ -7,6 +7,7 @@
 from modernrpc.auth.basic import http_basic_auth_login_required
 
 from db.columns.operations.alter import alter_columns_in_table
+from db.columns.operations.create import add_columns_to_table
 from db.columns.operations.drop import drop_columns_from_table
 from db.columns.operations.select import get_column_info_for_table
 from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions
@@ -74,11 +75,11 @@ def from_dict(cls, col_default):
             )
 
 
-class CreateableColumnInfo(TypedDict):
+class CreatableColumnInfo(TypedDict):
     """
-    Information about adding a new column.
+    Information needed to add a new column.
 
-    Only the `name` & `type` keys are required.
+    No keys are required.
 
     Attributes:
         name: The name of the column.
@@ -88,8 +89,8 @@ class CreateableColumnInfo(TypedDict):
         default: The default value.
         description: The description of the column.
     """
-    name: str
-    type: str
+    name: Optional[str]
+    type: Optional[str]
     type_options: Optional[TypeOptions]
     nullable: Optional[bool]
     default: Optional[ColumnDefault]
@@ -215,6 +216,36 @@ def list_(*, table_oid: int, database_id: int, **kwargs) -> ColumnListReturn:
     )
 
 
+@rpc_method(name="columns.add")
+@http_basic_auth_login_required
+@handle_rpc_exceptions
+def add(
+        *,
+        column_data_list: list[CreatableColumnInfo],
+        table_oid: int,
+        database_id: int,
+        **kwargs
+) -> list[int]:
+    """
+    Add columns to a table.
+
+    There are defaults for both the name and type of a column, and so
+    passing `[{}]` for `column_data_list` would add a single column of
+    type `CHARACTER VARYING`, with an auto-generated name.
+
+    Args:
+        column_data_list: A list describing desired columns to add.
+        table_oid: Identity of the table to which we'll add columns.
+        database_id: The Django id of the database containing the table.
+
+    Returns:
+        An array of the attnums of the new columns.
+    """
+    user = kwargs.get(REQUEST_KEY).user
+    with connect(database_id, user) as conn:
+        return add_columns_to_table(table_oid, column_data_list, conn)
+
+
 @rpc_method(name="columns.patch")
 @http_basic_auth_login_required
 @handle_rpc_exceptions

mathesar/rpc/constraints.py

@@ -4,5 +4,5 @@
 from typing import TypedDict
 
 
-class CreateableConstraintInfo(TypedDict):
+class CreatableConstraintInfo(TypedDict):
     pass

mathesar/rpc/tables.py

@@ -6,8 +6,8 @@
 from db.tables.operations.select import get_table_info, get_table
 from db.tables.operations.drop import drop_table_from_database
 from db.tables.operations.create import create_table_on_database
-from mathesar.rpc.columns import CreateableColumnInfo
-from mathesar.rpc.constraints import CreateableConstraintInfo
+from mathesar.rpc.columns import CreatableColumnInfo
+from mathesar.rpc.constraints import CreatableConstraintInfo
 from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions
 from mathesar.rpc.utils import connect
 
@@ -78,8 +78,8 @@ def add(
     table_name: str,
     schema_oid: int,
     database_id: int,
-    column_data_list: list[CreateableColumnInfo] = [],
-    constraint_data_list: list[CreateableConstraintInfo] = [],
+    column_data_list: list[CreatableColumnInfo] = [],
+    constraint_data_list: list[CreatableConstraintInfo] = [],
     comment: str = None,
     **kwargs
 ) -> int:

mathesar/tests/rpc/test_columns.py

@@ -171,6 +171,39 @@ def mock_column_alter(_table_oid, _column_data_list, conn):
     assert actual_result == 1
 
 
+def test_columns_add(rf, monkeypatch):
+    request = rf.post('/api/rpc/v0/', data={})
+    request.user = User(username='alice', password='pass1234')
+    table_oid = 23457
+    database_id = 2
+    column_data_list = [{"id": 3, "name": "newname"}]
+
+    @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_create(_table_oid, _column_data_list, conn):
+        if _table_oid != table_oid or _column_data_list != column_data_list:
+            raise AssertionError('incorrect parameters passed')
+        return [3, 4]
+
+    monkeypatch.setattr(columns, 'connect', mock_connect)
+    monkeypatch.setattr(columns, 'add_columns_to_table', mock_column_create)
+    actual_result = columns.add(
+        column_data_list=column_data_list,
+        table_oid=table_oid,
+        database_id=database_id,
+        request=request
+    )
+    assert actual_result == [3, 4]
+
+
 def test_columns_delete(rf, monkeypatch):
     request = rf.post('/api/rpc/v0/', data={})
     request.user = User(username='alice', password='pass1234')

mathesar/tests/rpc/test_endpoints.py

@@ -29,6 +29,11 @@
         "columns.patch",
         [user_is_authenticated]
     ),
+    (
+        columns.add,
+        "columns.add",
+        [user_is_authenticated]
+    ),
     (
         connections.add_from_known_connection,
         "connections.add_from_known_connection",