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

Add columns.add RPC function #3616

Merged
merged 7 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 63 additions & 26 deletions db/columns/operations/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
Expand All @@ -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,
Expand Down
37 changes: 17 additions & 20 deletions db/tests/columns/operations/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions docs/docs/api/rpc.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ To use an RPC function:
options:
members:
- list_
- add
- patch
- delete
- ColumnListReturn
Expand Down
37 changes: 32 additions & 5 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.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
Expand Down Expand Up @@ -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.
Expand All @@ -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]
Expand Down Expand Up @@ -215,6 +216,32 @@ 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.

Args:
column_data_list: A list describing desired columns to add.
Copy link
Member

@Anish9901 Anish9901 Jun 12, 2024

Choose a reason for hiding this comment

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

We should probably document the behavior when column_data_list is set to [{}](null)

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
Expand Down
2 changes: 1 addition & 1 deletion mathesar/rpc/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
from typing import TypedDict


class CreateableConstraintInfo(TypedDict):
class CreatableConstraintInfo(TypedDict):
pass
8 changes: 4 additions & 4 deletions mathesar/rpc/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
33 changes: 33 additions & 0 deletions mathesar/tests/rpc/test_columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
5 changes: 5 additions & 0 deletions mathesar/tests/rpc/test_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading