Implement schemas.add RPC method

conftest.py

@@ -14,7 +14,7 @@
 from db.types import install
 from db.sql import install as sql_install
 from db.schemas.operations.drop import drop_schema_via_name as drop_sa_schema
-from db.schemas.operations.create import create_schema as create_sa_schema
+from db.schemas.operations.create import create_schema_if_not_exists_via_sql_alchemy
 from db.schemas.utils import get_schema_oid_from_name, get_schema_name_from_oid
 
 from fixtures.utils import create_scoped_fixtures
@@ -210,7 +210,7 @@ def _create_schema(schema_name, engine, schema_mustnt_exist=True):
         if schema_mustnt_exist:
             assert schema_name not in created_schemas
         logger.debug(f'creating {schema_name}')
-        create_sa_schema(schema_name, engine, if_not_exists=True)
+        create_schema_if_not_exists_via_sql_alchemy(schema_name, engine)
         schema_oid = get_schema_oid_from_name(schema_name, engine)
         db_name = engine.url.database
         created_schemas_in_this_engine = created_schemas.setdefault(db_name, {})

db/schemas/operations/alter.py

@@ -27,10 +27,8 @@ def comment_on_schema(schema_name, engine, comment):
     Change description of a schema.
 
     Args:
-        schema_name: The name of the schema whose comment we will
-                     change.
-        comment: The new comment. Any quotes or special characters must
-                 be escaped.
+        schema_name: The name of the schema whose comment we will change.
+        comment: The new comment.
         engine: SQLAlchemy engine object for connecting.
 
     Returns:

db/schemas/operations/create.py

@@ -1,26 +1,53 @@
-from db.schemas.operations.alter import comment_on_schema
-from db.connection import execute_msar_func_with_engine
+from db.connection import execute_msar_func_with_engine, exec_msar_func
 
 
-def create_schema(schema_name, engine, comment=None, if_not_exists=False):
+def create_schema_via_sql_alchemy(schema_name, engine, description=None):
     """
-    Creates a schema.
+    Creates a schema using a SQLAlchemy engine.
 
     Args:
         schema_name: Name of the schema to create.
         engine: SQLAlchemy engine object for connecting.
-        comment: The new comment. Any quotes or special characters must
-                 be escaped.
-        if_not_exists: Whether to ignore an error if the schema does
-                       exist.
+        description: A new description to set on the schema.
+
+    If a schema already exists with the given name, this function will raise an error.
 
     Returns:
-        Returns a string giving the command that was run.
+        The integer oid of the newly created schema.
     """
-    result = execute_msar_func_with_engine(
-        engine, 'create_schema', schema_name, if_not_exists
+    return execute_msar_func_with_engine(
+        engine, 'create_schema', schema_name, description
     ).fetchone()[0]
 
-    if comment:
-        comment_on_schema(schema_name, engine, comment)
-    return result
+
+def create_schema_if_not_exists_via_sql_alchemy(schema_name, engine):
+    """
+    Ensure that a schema exists using a SQLAlchemy engine.
+
+    Args:
+        schema_name: Name of the schema to create.
+        engine: SQLAlchemy engine object for connecting.
+
+    Returns:
+        The integer oid of the newly created schema.
+    """
+    return execute_msar_func_with_engine(
+        engine, 'create_schema_if_not_exists', schema_name
+    ).fetchone()[0]
+
+
+def create_schema(schema_name, conn, description=None):
+    """
+    Create a schema using a psycopg connection.
+
+    Args:
+        schema_name: Name of the schema to create.
+        conn: a psycopg connection
+        description: A new description to set on the schema.
+
+    If a schema already exists with the given name, this function will raise an error.
+
+    Returns:
+        The integer oid of the newly created schema.
+    """
+    return exec_msar_func(conn, 'create_schema', schema_name, description).fetchone()[0]

db/sql/00_msar.sql

@@ -143,24 +143,27 @@ $$ LANGUAGE plpgsql;
 
 
 CREATE OR REPLACE FUNCTION msar.schema_exists(schema_name text) RETURNS boolean AS $$/*
-Return true if the given schema exists in the current database, false otherwise.
+Return true if the schema exists, false otherwise.
+
+Args :
+  sch_name: The name of the schema, UNQUOTED.
 */
 SELECT EXISTS (SELECT 1 FROM pg_namespace WHERE nspname=schema_name);
 $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;
 
 
-CREATE OR REPLACE FUNCTION __msar.get_schema_oid(sch_name text) RETURNS oid AS $$/*
-Return the OID of a schema, if it can be diretly found from a name.
+CREATE OR REPLACE FUNCTION msar.get_schema_oid(sch_name text) RETURNS oid AS $$/*
+Return the OID of a schema, or NULL if the schema does not exist.
 
 Args :
-  sch_name: The name of the schema.
+  sch_name: The name of the schema, UNQUOTED.
 */
-SELECT CASE WHEN msar.schema_exists(sch_name) THEN sch_name::regnamespace::oid END;
+SELECT oid FROM pg_namespace WHERE nspname=sch_name;
 $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;
 
 
 CREATE OR REPLACE FUNCTION __msar.get_schema_name(sch_id oid) RETURNS TEXT AS $$/*
-Return the name for a given schema, quoted as appropriate.
+Return the QUOTED name for a given schema.
 
 The schema *must* be in the pg_namespace table to use this function.
 
@@ -612,7 +615,7 @@ Args:
 SELECT jsonb_agg(prorettype::regtype::text)
 FROM pg_proc
 WHERE
-  pronamespace=__msar.get_schema_oid('mathesar_types')
+  pronamespace=msar.get_schema_oid('mathesar_types')
   AND proargtypes[0]=typ_id
   AND left(proname, 5) = 'cast_';
 $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;
@@ -885,8 +888,8 @@ __msar.comment_on_schema(sch_name text, comment_ text) RETURNS TEXT AS $$/*
 Change the description of a schema, returning command executed.
 
 Args:
-  sch_name: The quoted name of the schema whose comment we will change.
-  comment_: The new comment. Any quotes or special characters must be escaped.
+  sch_name: The QUOTED name of the schema whose comment we will change.
+  comment_: The new comment, QUOTED
 */
 DECLARE
   cmd_template text;
@@ -902,8 +905,8 @@ msar.comment_on_schema(sch_name text, comment_ text) RETURNS TEXT AS $$/*
 Change the description of a schema, returning command executed.
 
 Args:
-  sch_name: The quoted name of the schema whose comment we will change.
-  comment_: The new comment.
+  sch_name: The UNQUOTED name of the schema whose comment we will change.
+  comment_: The new comment, UNQUOTED
 */
 BEGIN
   RETURN __msar.comment_on_schema(quote_ident(sch_name), quote_literal(comment_));
@@ -916,7 +919,7 @@ Change the description of a schema, returning command executed.
 
 Args:
   sch_id: The OID of the schema.
-  comment_: The new comment.
+  comment_: The new comment, UNQUOTED
 */
 BEGIN
   RETURN __msar.comment_on_schema(__msar.get_schema_name(sch_id), quote_literal(comment_));
@@ -932,43 +935,54 @@ $$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;
 ----------------------------------------------------------------------------------------------------
 ----------------------------------------------------------------------------------------------------
 
+-- This gets rid of `msar.create_schema` as defined in Mathesar 0.1.7. We don't want that old
+-- function definition hanging around because it will get invoked when passing NULL as the second
+-- argument like `msar.create_schema('foo', NULL)`.
+DROP FUNCTION IF EXISTS msar.create_schema(text, boolean);
 
--- Create schema -----------------------------------------------------------------------------------
-
-CREATE OR REPLACE FUNCTION
-__msar.create_schema(sch_name text, if_not_exists boolean) RETURNS TEXT AS $$/*
-Create a schema, returning the command executed.
+CREATE OR REPLACE FUNCTION msar.create_schema_if_not_exists(sch_name text) RETURNS oid AS $$/*
+Ensure that a schema exists in the database.
 
 Args:
-  sch_name: A properly quoted name of the schema to be created
-  if_not_exists: Whether to ignore an error if the schema does exist
+  sch_name: the name of the schema to be created, UNQUOTED.
+
+Returns:
+  The integer OID of the schema
 */
-DECLARE
-  cmd_template text;
 BEGIN
-  IF if_not_exists
-  THEN
-    cmd_template := 'CREATE SCHEMA IF NOT EXISTS %s';
-  ELSE
-    cmd_template := 'CREATE SCHEMA %s';
-  END IF;
-  RETURN __msar.exec_ddl(cmd_template, sch_name);
+  EXECUTE 'CREATE SCHEMA IF NOT EXISTS ' || quote_ident(sch_name);
+  RETURN msar.get_schema_oid(sch_name);
 END;
-$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;
+$$ LANGUAGE plpgsql;
 
 
-CREATE OR REPLACE FUNCTION
-msar.create_schema(sch_name text, if_not_exists boolean) RETURNS TEXT AS $$/*
-Create a schema, returning the command executed.
+CREATE OR REPLACE FUNCTION msar.create_schema(
+  sch_name text,
+  description text DEFAULT ''
+) RETURNS oid AS $$/*
+Create a schema, possibly with a description.
+
+If a schema with the given name already exists, an exception will be raised.
 
 Args:
-  sch_name: An unquoted name of the schema to be created
-  if_not_exists: Whether to ignore an error if the schema does exist
+  sch_name: the name of the schema to be created, UNQUOTED.
+  description: (optional) A description for the schema, UNQUOTED.
+
+Returns:
+  The integer OID of the schema
+
+Note: This function does not support IF NOT EXISTS because it's simpler that way. I originally tried
+to support descriptions and if_not_exists in the same function, but as I discovered more edge cases
+and inconsistencies, it got too complex, and I didn't think we'd have a good enough use case for it.
 */
+DECLARE schema_oid oid;
 BEGIN
-  RETURN __msar.create_schema(quote_ident(sch_name), if_not_exists);
+  EXECUTE 'CREATE SCHEMA ' || quote_ident(sch_name);
+  schema_oid := msar.get_schema_oid(sch_name);
+  PERFORM msar.comment_on_schema(schema_oid, description);
+  RETURN schema_oid;
 END;
-$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;
+$$ LANGUAGE plpgsql;
 
 
 ----------------------------------------------------------------------------------------------------

db/sql/test_00_msar.sql

@@ -1183,17 +1183,38 @@ $$ LANGUAGE plpgsql;
 
 -- msar.schema_ddl --------------------------------------------------------------------------------
 
-CREATE OR REPLACE FUNCTION test_create_schema() RETURNS SETOF TEXT AS $$
+CREATE OR REPLACE FUNCTION test_create_schema_without_description() RETURNS SETOF TEXT AS $$
+DECLARE sch_oid oid;
 BEGIN
-  PERFORM msar.create_schema(
-    sch_name => 'create_schema'::text,
-    if_not_exists => false
-  );
-  RETURN NEXT has_schema('create_schema');
+  SELECT msar.create_schema('foo bar') INTO sch_oid;
+  RETURN NEXT has_schema('foo bar');
+  RETURN NEXT is(sch_oid, msar.get_schema_oid('foo bar'));
+  RETURN NEXT is(obj_description(sch_oid), NULL);
+END;
+$$ LANGUAGE plpgsql;
+
+
+CREATE OR REPLACE FUNCTION test_create_schema_with_description() RETURNS SETOF TEXT AS $$
+DECLARE sch_oid oid;
+BEGIN
+  SELECT msar.create_schema('foo bar', 'yay') INTO sch_oid;
+  RETURN NEXT has_schema('foo bar');
+  RETURN NEXT is(sch_oid, msar.get_schema_oid('foo bar'));
+  RETURN NEXT is(obj_description(sch_oid), 'yay');
 END;
 $$ LANGUAGE plpgsql;
 
 
+CREATE OR REPLACE FUNCTION test_create_schema_that_already_exists() RETURNS SETOF TEXT AS $t$
+DECLARE sch_oid oid;
+BEGIN
+  SELECT msar.create_schema('foo bar') INTO sch_oid;
+  RETURN NEXT throws_ok($$SELECT msar.create_schema('foo bar')$$, '42P06');
+  RETURN NEXT is(msar.create_schema_if_not_exists('foo bar'), sch_oid);
+END;
+$t$ LANGUAGE plpgsql;
+
+
 CREATE OR REPLACE FUNCTION __setup_drop_schema() RETURNS SETOF TEXT AS $$
 BEGIN
   CREATE SCHEMA drop_test_schema;

db/tables/operations/infer_types.py

@@ -5,7 +5,7 @@
 from db import constants
 from db.columns.base import MathesarColumn
 from db.columns.operations.infer_types import infer_column_type
-from db.schemas.operations.create import create_schema
+from db.schemas.operations.create import create_schema_if_not_exists_via_sql_alchemy
 from db.tables.operations.create import CreateTableAs
 from db.tables.operations.select import reflect_table
 from db.types.operations.convert import get_db_type_enum_from_class
@@ -43,7 +43,7 @@ def infer_table_column_types(schema, table_name, engine, metadata=None, columns_
     table = reflect_table(table_name, schema, engine, metadata=metadata)
 
     temp_name = TEMP_TABLE % (int(time()))
-    create_schema(TEMP_SCHEMA, engine, if_not_exists=True)
+    create_schema_if_not_exists_via_sql_alchemy(TEMP_SCHEMA, engine)
     with engine.begin() as conn:
         while engine.dialect.has_table(conn, temp_name, schema=TEMP_SCHEMA):
             temp_name = TEMP_TABLE.format(int(time()))

db/tests/schemas/operations/test_create.py

@@ -1,22 +1,16 @@
-import pytest
 from unittest.mock import patch
 import db.schemas.operations.create as sch_create
 
 
-@pytest.mark.parametrize(
-    "if_not_exists", [(True), (False), (None)]
-)
-def test_create_schema(engine_with_schema, if_not_exists):
+def test_create_schema_via_sql_alchemy(engine_with_schema):
     engine = engine_with_schema
     with patch.object(sch_create, 'execute_msar_func_with_engine') as mock_exec:
-        sch_create.create_schema(
+        sch_create.create_schema_via_sql_alchemy(
             schema_name='new_schema',
             engine=engine,
-            comment=None,
-            if_not_exists=if_not_exists
+            description=None,
         )
     call_args = mock_exec.call_args_list[0][0]
     assert call_args[0] == engine
     assert call_args[1] == "create_schema"
     assert call_args[2] == "new_schema"
-    assert call_args[3] == if_not_exists or False

db/types/install.py

@@ -1,12 +1,12 @@
 from db.types.custom import email, money, multicurrency, uri, json_array, json_object
 from db.constants import TYPES_SCHEMA
-from db.schemas.operations.create import create_schema
+from db.schemas.operations.create import create_schema_if_not_exists_via_sql_alchemy
 from db.types.operations.cast import install_all_casts
 import psycopg
 
 
-def create_type_schema(engine):
-    create_schema(TYPES_SCHEMA, engine, if_not_exists=True)
+def create_type_schema(engine) -> None:
+    create_schema_if_not_exists_via_sql_alchemy(TYPES_SCHEMA, engine)
 
 
 def install_mathesar_on_database(engine):
@@ -24,4 +24,6 @@ def install_mathesar_on_database(engine):
 def uninstall_mathesar_from_database(engine):
     conn_str = str(engine.url)
     with psycopg.connect(conn_str) as conn:
+        # TODO: Clean up this code so that it references all the schemas in our
+        # `INTERNAL_SCHEMAS` constant.
         conn.execute(f"DROP SCHEMA IF EXISTS __msar, msar, {TYPES_SCHEMA} CASCADE")

docs/docs/api/rpc.md

@@ -52,6 +52,7 @@ To use an RPC function:
     options:
       members:
       - list_
+      - add
       - delete
       - SchemaInfo