Merge branch 'develop' into permissions_remodel_new

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;
 
 
 ----------------------------------------------------------------------------------------------------
@@ -1057,7 +1071,7 @@ Args:
   new_tab_name: unquoted, unqualified table name
 */
 BEGIN
-  RETURN __msar.rename_table(__msar.get_relation_name(tab_id), quote_ident(new_tab_name));
+  RETURN __msar.rename_table(msar.get_relation_name_or_null(tab_id), quote_ident(new_tab_name));
 END;
 $$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;
 
@@ -1089,8 +1103,12 @@ Args:
   tab_name: The qualified, quoted name of the table whose comment we will change.
   comment_: The new comment. Any quotes or special characters must be escaped.
 */
-SELECT __msar.exec_ddl('COMMENT ON TABLE %s IS %s', tab_name, comment_);
-$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;
+DECLARE
+  comment_or_null text := COALESCE(comment_, 'NULL');
+BEGIN
+RETURN __msar.exec_ddl('COMMENT ON TABLE %s IS %s', tab_name, comment_or_null);
+END;
+$$ LANGUAGE plpgsql;
 
 
 CREATE OR REPLACE FUNCTION
@@ -1101,8 +1119,8 @@ Args:
   tab_id: The OID of the table whose comment we will change.
   comment_: The new comment.
 */
-SELECT __msar.comment_on_table(__msar.get_relation_name(tab_id), quote_literal(comment_));
-$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;
+SELECT __msar.comment_on_table(msar.get_relation_name_or_null(tab_id), quote_literal(comment_));
+$$ LANGUAGE SQL;
 
 
 CREATE OR REPLACE FUNCTION
@@ -1118,10 +1136,40 @@ SELECT __msar.comment_on_table(
   msar.get_fully_qualified_object_name(sch_name, tab_name),
   quote_literal(comment_)
 );
-$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;
+$$ LANGUAGE SQL;
+
 
+-- Alter table -------------------------------------------------------------------------------------
+CREATE OR REPLACE FUNCTION
+msar.alter_table(tab_id oid, tab_alters jsonb) RETURNS text AS $$/*
+Alter the name, description, or columns of a table, returning name of the altered table.
+
+Args:
+  tab_id: The OID of the table whose columns we'll alter.
+  tab_alters: a JSONB describing the alterations to make.
+
+  The tab_alters should have the form:
+  {
+    "name": <str>,
+    "description": <str>
+    "columns": <col_alters>,
+  }
+*/
+DECLARE
+  new_tab_name text;
+  comment text;
+  col_alters jsonb;
+BEGIN
+  new_tab_name := tab_alters->>'name';
+  comment := tab_alters->>'description';
+  col_alters := tab_alters->'columns';
+  PERFORM msar.rename_table(tab_id, new_tab_name);
+  PERFORM msar.comment_on_table(tab_id, comment);
+  PERFORM msar.alter_columns(tab_id, col_alters);
+  RETURN msar.get_relation_name_or_null(tab_id);
+END;
+$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;
 
--- Alter Table: LEFT IN PYTHON (for now) -----------------------------------------------------------
 
 ----------------------------------------------------------------------------------------------------
 ----------------------------------------------------------------------------------------------------

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;