From 115485d73718117df2f794e6064aadfc6df3be35 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Thu, 18 Apr 2024 15:01:39 +0800 Subject: [PATCH 01/28] add initial db connection getting stub --- db/connection.py | 15 +++++++++++++++ mathesar/database/base.py | 17 +++++++++++++++++ mathesar/rpc/utils.py | 13 +++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 mathesar/rpc/utils.py diff --git a/db/connection.py b/db/connection.py index 2b546256d3..5af890107f 100644 --- a/db/connection.py +++ b/db/connection.py @@ -40,6 +40,21 @@ def execute_msar_func_with_psycopg2_conn(conn, func_name, *args): return conn.execute(stmt) +def exec_msar_func(conn, func_name, *args): + """ + Execute an msar function using a psycopg (3) connection. + + Args: + conn: a psycopg connection + func_name: The unqualified msar_function name (danger; not sanitized) + *args: The list of parameters to pass + """ + # Returns a cursor + return conn.execute( + f"SELECT msar.{func_name}({','.join(['%s']*len(args))})", args + ) + + def load_file_with_engine(engine, file_handle): """Run an SQL script from a file, using psycopg.""" conn_str = str(engine.url) diff --git a/mathesar/database/base.py b/mathesar/database/base.py index d6384d3d76..c07552be26 100644 --- a/mathesar/database/base.py +++ b/mathesar/database/base.py @@ -1,3 +1,4 @@ +import psycopg from db import engine @@ -18,3 +19,19 @@ def _get_credentials_for_db_model(db_model): database=db_model.db_name, port=db_model.port, ) + + +def get_psycopg_connection(db_model): + """ + Get a psycopg connection, given a Database model. + + Args: + db_model: The Django model corresponding to the Database. + """ + return psycopg.connect( + host=db_model.host, + port=db_model.port, + dbname=db_model.db_name, + user=db_model.username, + password=db_model.password, + ) diff --git a/mathesar/rpc/utils.py b/mathesar/rpc/utils.py new file mode 100644 index 0000000000..7bc33919d0 --- /dev/null +++ b/mathesar/rpc/utils.py @@ -0,0 +1,13 @@ +from mathesar.database.base import get_psycopg_connection +from mathesar.models.base import Database + + +def connect(db_id): + """ + Return a psycopg connection, given a Database model id. + + Args: + db_id: The Django id corresponding to the Database. + """ + db_model = Database.current_objects.get(id=db_id) + return get_psycopg_connection(db_model) From 518b7e15d09096a700d73be315f028d8acef0f96 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Thu, 25 Apr 2024 16:35:17 +0800 Subject: [PATCH 02/28] first pass at functions to build the type_options JSON --- db/sql/0_msar.sql | 77 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/db/sql/0_msar.sql b/db/sql/0_msar.sql index e16bf53246..65a7db3f1b 100644 --- a/db/sql/0_msar.sql +++ b/db/sql/0_msar.sql @@ -468,6 +468,83 @@ AND attrelid = msar.get_relation_oid(sch_name, rel_name); $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; +CREATE OR REPLACE FUNCTION +msar.get_interval_fields(typ_mod integer) RETURNS text AS $$/* +Return the string giving the fields for an interval typmod integer. + +This logic is ported from the relevant PostgreSQL source code, reimplemented in SQL. See: +https://doxygen.postgresql.org/backend_2utils_2adt_2timestamp_8c.html + +Args: + typ_mod: The atttypmod from the pg_attribute table. Should be valid for the interval type. +*/ +SELECT CASE (typ_mod >> 16 & 32767) + WHEN 1 << 2 THEN 'year' + WHEN 1 << 1 THEN 'month' + WHEN 1 << 3 THEN 'day' + WHEN 1 << 10 THEN 'hour' + WHEN 1 << 11 THEN 'minute' + WHEN 1 << 12 THEN 'second' + WHEN (1 << 2) | (1 << 1) THEN 'year to month' + WHEN (1 << 3) | (1 << 10) THEN 'day to hour' + WHEN (1 << 3) | (1 << 10) | (1 << 11) THEN 'day to minute' + WHEN (1 << 3) | (1 << 10) | (1 << 11) | (1 << 12) THEN 'day to second' + WHEN (1 << 10) | (1 << 11) THEN 'hour to minute' + WHEN (1 << 10) | (1 << 11) | (1 << 12) THEN 'hour to second' + WHEN (1 << 11) | (1 << 12) THEN 'minute to second' +END; +$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; + + +CREATE OR REPLACE FUNCTION +msar.get_type_options(typ_id oid, typ_mod integer, typ_ndims integer) RETURNS jsonb AS $$/* +Return the type options calculated from a type, typmod pair. + +This function uses a number of hard-coded constants. + +Args: + typ_id: The OID of the type. + typ_mod: The integer corresponding to the type options; see pg_attribute catalog table. +*/ +SELECT CASE + -- Columns with no type options store -1 in the atttypmod column. + WHEN typ_id = 'numeric'::regtype::oid OR typ_id='numeric[]'::regtype::oid THEN + jsonb_build_object( + -- This calculation is modified from the relevant PostgreSQL source code. See + -- https://doxygen.postgresql.org/backend_2utils_2adt_2numeric_8c.html + 'precision', ((nullif(typ_mod, -1) - 4) >> 16) & 65535, + -- This calculation is also from the source code. + 'scale', (((nullif(typ_mod, -1) - 4) & 2047) # 1024) - 1024 + ) + WHEN typ_id = 'interval'::regtype::oid OR typ_id = '_interval'::regtype::oid THEN + jsonb_build_object( + 'precision', NULLIF(typ_mod & 65535, 65535), + 'fields', msar.get_interval_fields(typ_mod) + ) + WHEN ARRAY[typ_id] <@ ARRAY[ + 'bpchar'::regtype::oid, 'varchar'::regtype::oid, + '_bpchar'::regtype::oid, '_varchar'::regtype::oid + ] THEN + jsonb_build_object( + 'length', nullif(typ_mod, -1) - 4 + ) + WHEN ARRAY[typ_id] <@ ARRAY[ + 'bit'::regtype::oid, 'varbit'::regtype::oid, + '_bit'::regtype::oid, '_varbit'::regtype::oid, + 'time'::regtype::oid, 'timetz'::regtype::oid, + '_time'::regtype::oid, '_timetz'::regtype::oid, + 'timestamp'::regtype::oid, 'timestamptz'::regtype::oid, + '_timestamp'::regtype::oid, '_timestamptz'::regtype::oid + ] THEN + -- For all these types, the typmod is equal to the precision. + jsonb_build_object( + 'precision', nullif(typ_mod, -1) + ) + ELSE jsonb_build_object() +END || jsonb_build_object('array', typ_ndims>0); +$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; + + CREATE OR REPLACE FUNCTION msar.column_exists(tab_id oid, col_name text) RETURNS boolean AS $$/* Return true if the given column exists in the table, false otherwise. */ From 6cb7ba089879c5f03a8dcc16772ba8e64c049074 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 26 Apr 2024 13:00:12 +0800 Subject: [PATCH 03/28] add column info getting function and utils --- db/sql/0_msar.sql | 65 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/db/sql/0_msar.sql b/db/sql/0_msar.sql index 65a7db3f1b..5c7b4249a4 100644 --- a/db/sql/0_msar.sql +++ b/db/sql/0_msar.sql @@ -545,6 +545,71 @@ END || jsonb_build_object('array', typ_ndims>0); $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; +CREATE OR REPLACE FUNCTION msar.get_valid_target_type_strings(typ_id oid) RETURNS jsonb AS $$/* +TODO +*/ +SELECT jsonb_agg(prorettype::regtype::text) +FROM pg_proc +WHERE + pronamespace='mathesar_types'::regnamespace::oid + AND proargtypes[0]=typ_id + AND left(proname, 5) = 'cast_'; +$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; + + +CREATE OR REPLACE FUNCTION msar.has_dependents(rel_id oid, att_id smallint) RETURNS boolean AS $$/* +Return a boolean according to whether the column identified by the given oid, attnum pair is +referenced (i.e., would dropping that column require CASCADE?). + +Args: + rel_id: The relation of the attribute. + att_id: The attnum of the attribute in the relation. +*/ +SELECT EXISTS ( + SELECT 1 FROM pg_depend WHERE refobjid=rel_id AND refobjsubid=att_id AND deptype='n' +); +$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; + + +CREATE OR REPLACE FUNCTION msar.get_column_info(tab_id regclass) RETURNS jsonb AS $$/* +TODO +*/ +SELECT jsonb_agg( + jsonb_build_object( + 'id', attnum, + 'name', attname, + 'type', rtrim(atttypid::regtype::text, '[]'), + 'type_options', msar.get_type_options(atttypid, atttypmod, attndims), + 'nullable', NOT attnotnull, + 'primary_key', COALESCE(pgi.indisprimary, false), + 'valid_target_types', msar.get_valid_target_type_strings(atttypid), + 'default', + nullif( + jsonb_strip_nulls( + jsonb_build_object( + 'value', + CASE + WHEN attidentity='' THEN pg_get_expr(adbin, tab_id) + ELSE 'identity' + END, + 'is_dynamic', msar.is_default_possibly_dynamic(tab_id, attnum) + ) + ), + jsonb_build_object() + ), + 'has_dependents', msar.has_dependents(tab_id, attnum), + 'description', msar.col_description(tab_id, attnum) + ) +) +FROM pg_attribute pga + LEFT JOIN pg_index pgi ON pga.attrelid=pgi.indrelid AND pga.attnum=ANY(pgi.indkey) + LEFT JOIN pg_attrdef pgd ON pga.attrelid=pgd.adrelid AND pga.attnum=pgd.adnum +WHERE pga.attrelid=tab_id AND pga.attnum > 0 and NOT attisdropped; +$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; + + + + CREATE OR REPLACE FUNCTION msar.column_exists(tab_id oid, col_name text) RETURNS boolean AS $$/* Return true if the given column exists in the table, false otherwise. */ From 96338c7b21a2691567416667368e6e9dd28668f3 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 26 Apr 2024 14:17:57 +0800 Subject: [PATCH 04/28] use different style to denote arrays, clean up code --- db/sql/0_msar.sql | 72 +++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 37 deletions(-) diff --git a/db/sql/0_msar.sql b/db/sql/0_msar.sql index 5c7b4249a4..1ac1f87986 100644 --- a/db/sql/0_msar.sql +++ b/db/sql/0_msar.sql @@ -506,42 +506,40 @@ Args: typ_id: The OID of the type. typ_mod: The integer corresponding to the type options; see pg_attribute catalog table. */ -SELECT CASE - -- Columns with no type options store -1 in the atttypmod column. - WHEN typ_id = 'numeric'::regtype::oid OR typ_id='numeric[]'::regtype::oid THEN - jsonb_build_object( - -- This calculation is modified from the relevant PostgreSQL source code. See - -- https://doxygen.postgresql.org/backend_2utils_2adt_2numeric_8c.html - 'precision', ((nullif(typ_mod, -1) - 4) >> 16) & 65535, - -- This calculation is also from the source code. - 'scale', (((nullif(typ_mod, -1) - 4) & 2047) # 1024) - 1024 - ) - WHEN typ_id = 'interval'::regtype::oid OR typ_id = '_interval'::regtype::oid THEN - jsonb_build_object( - 'precision', NULLIF(typ_mod & 65535, 65535), - 'fields', msar.get_interval_fields(typ_mod) - ) - WHEN ARRAY[typ_id] <@ ARRAY[ - 'bpchar'::regtype::oid, 'varchar'::regtype::oid, - '_bpchar'::regtype::oid, '_varchar'::regtype::oid - ] THEN - jsonb_build_object( - 'length', nullif(typ_mod, -1) - 4 - ) - WHEN ARRAY[typ_id] <@ ARRAY[ - 'bit'::regtype::oid, 'varbit'::regtype::oid, - '_bit'::regtype::oid, '_varbit'::regtype::oid, - 'time'::regtype::oid, 'timetz'::regtype::oid, - '_time'::regtype::oid, '_timetz'::regtype::oid, - 'timestamp'::regtype::oid, 'timestamptz'::regtype::oid, - '_timestamp'::regtype::oid, '_timestamptz'::regtype::oid - ] THEN - -- For all these types, the typmod is equal to the precision. - jsonb_build_object( - 'precision', nullif(typ_mod, -1) - ) - ELSE jsonb_build_object() -END || jsonb_build_object('array', typ_ndims>0); +SELECT nullif( + CASE + WHEN typ_id = ANY('{numeric, _numeric}'::regtype[]) THEN + jsonb_build_object( + -- This calculation is modified from the relevant PostgreSQL source code. See + -- https://doxygen.postgresql.org/backend_2utils_2adt_2numeric_8c.html + 'precision', ((nullif(typ_mod, -1) - 4) >> 16) & 65535, + -- This calculation is also from the source code. + 'scale', (((nullif(typ_mod, -1) - 4) & 2047) # 1024) - 1024 + ) + WHEN typ_id = ANY('{interval, _interval}'::regtype[]) THEN + jsonb_build_object( + 'precision', nullif(typ_mod & 65535, 65535), + 'fields', msar.get_interval_fields(typ_mod) + ) + WHEN typ_id = ANY('{bpchar, _bpchar, varchar, _varchar}'::regtype[]) THEN + jsonb_build_object('length', nullif(typ_mod, -1) - 4) + WHEN typ_id = ANY( + '{bit, varbit, time, timetz, timestamp, timestamptz}'::regtype[] + || '{_bit, _varbit, _time, _timetz, _timestamp, _timestamptz}'::regtype[] + ) THEN + -- For all these types, the typmod is equal to the precision. + jsonb_build_object( + 'precision', nullif(typ_mod, -1) + ) + ELSE jsonb_build_object() + END + || CASE + WHEN typ_ndims>0 THEN + jsonb_build_object('item_type', typ_id::regtype) + ELSE '{}' + END, + '{}' +) $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; @@ -578,7 +576,7 @@ SELECT jsonb_agg( jsonb_build_object( 'id', attnum, 'name', attname, - 'type', rtrim(atttypid::regtype::text, '[]'), + 'type', CASE WHEN attndims>0 THEN '_array' ELSE atttypid::regtype::text END, 'type_options', msar.get_type_options(atttypid, atttypmod, attndims), 'nullable', NOT attnotnull, 'primary_key', COALESCE(pgi.indisprimary, false), From 2904112f3cd7c56a9be67be347747780f457d9c0 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 29 Apr 2024 15:58:08 +0800 Subject: [PATCH 05/28] add/improve docstrings of column info getter SQL functions --- db/sql/0_msar.sql | 52 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/db/sql/0_msar.sql b/db/sql/0_msar.sql index 1ac1f87986..ce97fc16c2 100644 --- a/db/sql/0_msar.sql +++ b/db/sql/0_msar.sql @@ -472,7 +472,8 @@ CREATE OR REPLACE FUNCTION msar.get_interval_fields(typ_mod integer) RETURNS text AS $$/* Return the string giving the fields for an interval typmod integer. -This logic is ported from the relevant PostgreSQL source code, reimplemented in SQL. See: +This logic is ported from the relevant PostgreSQL source code, reimplemented in SQL. See the +`intervaltypmodout` function at https://doxygen.postgresql.org/backend_2utils_2adt_2timestamp_8c.html Args: @@ -497,23 +498,31 @@ $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; CREATE OR REPLACE FUNCTION -msar.get_type_options(typ_id oid, typ_mod integer, typ_ndims integer) RETURNS jsonb AS $$/* +msar.get_type_options(typ_id regtype, typ_mod integer, typ_ndims integer) RETURNS jsonb AS $$/* Return the type options calculated from a type, typmod pair. -This function uses a number of hard-coded constants. +This function uses a number of hard-coded constants. The form of the returned object is determined +by the input type, but the keys will be a subset of: + precision: the precision of a numeric or interval type. See PostgreSQL docs for details. + scale: the scale of a numeric type + fields: See PostgreSQL documentation of the `interval` type. + length: Applies to "text" types where the user can specify the length. + item_type: Gives the type of array members for array-types Args: - typ_id: The OID of the type. + typ_id: an OID or valid type representing string will work here. typ_mod: The integer corresponding to the type options; see pg_attribute catalog table. + typ_ndims: Used to determine whether the type is actually an array without an extra join. */ SELECT nullif( CASE WHEN typ_id = ANY('{numeric, _numeric}'::regtype[]) THEN jsonb_build_object( - -- This calculation is modified from the relevant PostgreSQL source code. See + -- This calculation is modified from the relevant PostgreSQL source code. See the function + -- numeric_typmod_precision(int32) at -- https://doxygen.postgresql.org/backend_2utils_2adt_2numeric_8c.html 'precision', ((nullif(typ_mod, -1) - 4) >> 16) & 65535, - -- This calculation is also from the source code. + -- This calculation is from numeric_typmod_scale(int32) at the same location 'scale', (((nullif(typ_mod, -1) - 4) & 2047) # 1024) - 1024 ) WHEN typ_id = ANY('{interval, _interval}'::regtype[]) THEN @@ -522,6 +531,7 @@ SELECT nullif( 'fields', msar.get_interval_fields(typ_mod) ) WHEN typ_id = ANY('{bpchar, _bpchar, varchar, _varchar}'::regtype[]) THEN + -- For char and varchar types, the typemod is equal to 4 more than the set length. jsonb_build_object('length', nullif(typ_mod, -1) - 4) WHEN typ_id = ANY( '{bit, varbit, time, timetz, timestamp, timestamptz}'::regtype[] @@ -543,8 +553,11 @@ SELECT nullif( $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; -CREATE OR REPLACE FUNCTION msar.get_valid_target_type_strings(typ_id oid) RETURNS jsonb AS $$/* -TODO +CREATE OR REPLACE FUNCTION msar.get_valid_target_type_strings(typ_id regtype) RETURNS jsonb AS $$/* +Given a source type, return the target types for which Mathesar provides a casting function. + +Args: + typ_id: The type we're casting from. */ SELECT jsonb_agg(prorettype::regtype::text) FROM pg_proc @@ -570,7 +583,26 @@ $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; CREATE OR REPLACE FUNCTION msar.get_column_info(tab_id regclass) RETURNS jsonb AS $$/* -TODO +Given a table identifier, return an array of objects describing the columns of the table. + +Each returned JSON object in the array will have the form: + { + "id": , + "name": , + "type": , + "type_options": , + "nullable": , + "primary_key": , + "valid_target_types": [, , ..., ] + "default": {"value": , "is_dynamic": }, + "has_dependents": , + "description": + } + +The `type_options` object is described in the docstring of `msar.get_type_options`. The `default` +object has the keys: + value: A string giving the value (as an SQL expression) of the default. + is_dynamic: A boolean giving whether the default is (likely to be) dynamic. */ SELECT jsonb_agg( jsonb_build_object( @@ -606,8 +638,6 @@ WHERE pga.attrelid=tab_id AND pga.attnum > 0 and NOT attisdropped; $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; - - CREATE OR REPLACE FUNCTION msar.column_exists(tab_id oid, col_name text) RETURNS boolean AS $$/* Return true if the given column exists in the table, false otherwise. */ From 66ff45c3efcfd0ceb267ed78397ba0311204b183 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 29 Apr 2024 17:06:13 +0800 Subject: [PATCH 06/28] remove dependency on mathesar_types existing --- db/sql/0_msar.sql | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/db/sql/0_msar.sql b/db/sql/0_msar.sql index ce97fc16c2..b3b2ff66fb 100644 --- a/db/sql/0_msar.sql +++ b/db/sql/0_msar.sql @@ -131,6 +131,23 @@ Wraps the `?` jsonb operator for improved readability. $$ 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. +*/ +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. + +Args : + sch_name: The name of the schema. +*/ +SELECT CASE WHEN msar.schema_exists(sch_name) THEN sch_name::regnamespace::oid END; +$$ 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. @@ -559,10 +576,11 @@ Given a source type, return the target types for which Mathesar provides a casti Args: typ_id: The type we're casting from. */ + SELECT jsonb_agg(prorettype::regtype::text) FROM pg_proc WHERE - pronamespace='mathesar_types'::regnamespace::oid + pronamespace=__msar.get_schema_oid('mathesar_types') AND proargtypes[0]=typ_id AND left(proname, 5) = 'cast_'; $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; @@ -645,13 +663,6 @@ SELECT EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid=tab_id AND attname=col_ $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; -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. -*/ -SELECT EXISTS (SELECT 1 FROM pg_namespace WHERE nspname=schema_name); -$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; - - ---------------------------------------------------------------------------------------------------- ---------------------------------------------------------------------------------------------------- -- ROLE MANIPULATION FUNCTIONS From c2e9df5c022ae9028900edfdee5faafcbe342536 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 29 Apr 2024 17:38:49 +0800 Subject: [PATCH 07/28] add SQL tests for interval field builder --- db/sql/test_0_msar.sql | 78 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/db/sql/test_0_msar.sql b/db/sql/test_0_msar.sql index e21f4712f6..3bff3ac770 100644 --- a/db/sql/test_0_msar.sql +++ b/db/sql/test_0_msar.sql @@ -2079,3 +2079,81 @@ BEGIN RETURN NEXT schema_privs_are ('__msar', 'Ro"\bert''); DROP SCHEMA public;', ARRAY['USAGE']); END; $$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION setup_interval_fields() RETURNS SETOF TEXT AS $$ +BEGIN +CREATE TABLE manytypes ( + id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, + ivl_plain interval, + ivl_yr interval year, + ivl_mo interval month, + ivl_dy interval day, + ivl_hr interval hour, + ivl_mi interval minute, + ivl_se interval second, + ivl_ye_mo interval year to month, + ivl_dy_hr interval day to hour, + ivl_dy_mi interval day to minute, + ivl_dy_se interval day to second, + ivl_hr_mi interval hour to minute, + ivl_hr_se interval hour to second, + ivl_mi_se interval minute to second, + ivl_se_0 interval second(0), + ivl_se_3 interval second(3), + ivl_se_6 interval second(6), + ivl_dy_se0 interval day to second(0), + ivl_dy_se3 interval day to second(3), + ivl_dy_se6 interval day to second(6), + ivl_hr_se0 interval hour to second(0), + ivl_hr_se3 interval hour to second(3), + ivl_hr_se6 interval hour to second(6), + ivl_mi_se0 interval minute to second(0), + ivl_mi_se3 interval minute to second(3), + ivl_mi_se6 interval minute to second(6) +); +END; +$$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_get_interval_fields() RETURNS SETOF TEXT AS $$ +BEGIN + RETURN NEXT results_eq( + $h$ + SELECT msar.get_interval_fields(atttypmod) + FROM pg_attribute + WHERE attrelid='manytypes'::regclass AND atttypid='interval'::regtype + ORDER BY attnum; + $h$, + $w$ + VALUES + (NULL), + ('year'), + ('month'), + ('day'), + ('hour'), + ('minute'), + ('second'), + ('year to month'), + ('day to hour'), + ('day to minute'), + ('day to second'), + ('hour to minute'), + ('hour to second'), + ('minute to second'), + ('second'), + ('second'), + ('second'), + ('day to second'), + ('day to second'), + ('day to second'), + ('hour to second'), + ('hour to second'), + ('hour to second'), + ('minute to second'), + ('minute to second'), + ('minute to second') + $w$ + ); +END; +$$ LANGUAGE plpgsql; From a6c2983b81e00a87f60153a35b62fede32a56155 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 29 Apr 2024 18:24:47 +0800 Subject: [PATCH 08/28] test get_type_options function, fix bug --- db/sql/0_msar.sql | 3 +- db/sql/test_0_msar.sql | 73 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/db/sql/0_msar.sql b/db/sql/0_msar.sql index b3b2ff66fb..45590aa3c4 100644 --- a/db/sql/0_msar.sql +++ b/db/sql/0_msar.sql @@ -562,7 +562,8 @@ SELECT nullif( END || CASE WHEN typ_ndims>0 THEN - jsonb_build_object('item_type', typ_id::regtype) + -- This string wrangling is debatably dubious, but avoids a slow join. + jsonb_build_object('item_type', rtrim(typ_id::regtype::text, '[]')) ELSE '{}' END, '{}' diff --git a/db/sql/test_0_msar.sql b/db/sql/test_0_msar.sql index 3bff3ac770..6e9bebdf65 100644 --- a/db/sql/test_0_msar.sql +++ b/db/sql/test_0_msar.sql @@ -2110,7 +2110,15 @@ CREATE TABLE manytypes ( ivl_hr_se6 interval hour to second(6), ivl_mi_se0 interval minute to second(0), ivl_mi_se3 interval minute to second(3), - ivl_mi_se6 interval minute to second(6) + ivl_mi_se6 interval minute to second(6), + -- Below here is less throrough, more ad-hoc + ivl_plain_arr interval[], + ivl_mi_se6_arr interval minute to second(6)[], + num_plain numeric, + num_8 numeric(8), + num_17_2 numeric(17, 2), + num_plain_arr numeric[], + num_17_2_arr numeric(17, 2)[] ); END; $$ LANGUAGE plpgsql; @@ -2157,3 +2165,66 @@ BEGIN ); END; $$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_get_type_options() RETURNS SETOF TEXT AS $$ +BEGIN + RETURN NEXT is(msar.get_type_options(atttypid, atttypmod, attndims), NULL) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='id'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"fields": null, "precision": null}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='ivl_plain'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"fields": "day to second", "precision": null}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='ivl_dy_se'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"fields": "second", "precision": 3}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='ivl_se_3'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"fields": "hour to second", "precision": 0}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='ivl_hr_se_0'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"fields": null, "precision": null, "item_type": "interval"}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='ivl_plain_arr'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"fields": "minute to second", "precision": 6, "item_type": "interval"}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='ivl_mi_se6_arr'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": null, "scale": null}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='num_plain'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 8, "scale": 0}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='num_8'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 17, "scale": 2}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='num_17_2'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": null, "scale": null, "item_type": "numeric"}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='num_plain_arr'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 17, "scale": 2, "item_type": "numeric"}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='num_17_2_arr'; +END; +$$ LANGUAGE plpgsql; From cee460bfe79fe089e8164880295db0219662d028 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 30 Apr 2024 09:48:17 +0800 Subject: [PATCH 09/28] add tests for textual type options --- db/sql/test_0_msar.sql | 47 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/db/sql/test_0_msar.sql b/db/sql/test_0_msar.sql index 6e9bebdf65..6325ec1d7b 100644 --- a/db/sql/test_0_msar.sql +++ b/db/sql/test_0_msar.sql @@ -2085,6 +2085,7 @@ CREATE OR REPLACE FUNCTION setup_interval_fields() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE manytypes ( id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, + -- To fend off likely typos, we check many combinations of field and precision settings. ivl_plain interval, ivl_yr interval year, ivl_mo interval month, @@ -2113,12 +2114,19 @@ CREATE TABLE manytypes ( ivl_mi_se6 interval minute to second(6), -- Below here is less throrough, more ad-hoc ivl_plain_arr interval[], - ivl_mi_se6_arr interval minute to second(6)[], + ivl_mi_se6_arr interval minute to second(6)[2][2], num_plain numeric, num_8 numeric(8), num_17_2 numeric(17, 2), num_plain_arr numeric[], - num_17_2_arr numeric(17, 2)[] + num_17_2_arr numeric(17, 2)[], + var_plain varchar, + var_16 varchar(16), + var_255 varchar(255), + cha_1 character, + cha_20 character(20), + var_16_arr varchar(16)[], + cha_20_arr character(20)[][] ); END; $$ LANGUAGE plpgsql; @@ -2226,5 +2234,40 @@ BEGIN '{"precision": 17, "scale": 2, "item_type": "numeric"}'::jsonb ) FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='num_17_2_arr'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"length": null}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='var_plain'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"length": 16}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='var_16'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"length": 255}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='var_255'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"length": 1}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='cha_1'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"length": 20}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='cha_20'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"length": 16, "item_type": "character varying"}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='var_16_arr'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"length": 20, "item_type": "character"}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='cha_20_arr'; END; $$ LANGUAGE plpgsql; From 1d465b8c82aaa069e4bde67285af587f67b4ab55 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 30 Apr 2024 10:30:51 +0800 Subject: [PATCH 10/28] move some setup functions into callers for efficiency --- db/sql/test_0_msar.sql | 137 ++++++++++++++++++++++++++--------------- 1 file changed, 89 insertions(+), 48 deletions(-) diff --git a/db/sql/test_0_msar.sql b/db/sql/test_0_msar.sql index 6325ec1d7b..c5cecb832c 100644 --- a/db/sql/test_0_msar.sql +++ b/db/sql/test_0_msar.sql @@ -3,7 +3,7 @@ CREATE EXTENSION IF NOT EXISTS pgtap; -- msar.drop_columns ------------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION setup_drop_columns() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_drop_columns() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE atable (dodrop1 integer, dodrop2 integer, dontdrop text); END; @@ -14,6 +14,7 @@ CREATE OR REPLACE FUNCTION test_drop_columns_oid() RETURNS SETOF TEXT AS $$ DECLARE rel_id oid; BEGIN + PERFORM __setup_drop_columns(); rel_id := 'atable'::regclass::oid; PERFORM msar.drop_columns(rel_id, 1, 2); RETURN NEXT has_column( @@ -31,6 +32,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_columns_names() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_drop_columns(); PERFORM msar.drop_columns('public', 'atable', 'dodrop1', 'dodrop2'); RETURN NEXT has_column( 'atable', 'dontdrop', 'Dropper keeps correct columns' @@ -47,7 +49,7 @@ $$ LANGUAGE plpgsql; -- msar.drop_table --------------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION setup_drop_tables() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_drop_tables() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE dropme (id SERIAL PRIMARY KEY, col1 integer); END; @@ -57,6 +59,7 @@ CREATE OR REPLACE FUNCTION test_drop_table_oid() RETURNS SETOF TEXT AS $$ DECLARE rel_id oid; BEGIN + PERFORM __setup_drop_tables(); rel_id := 'dropme'::regclass::oid; PERFORM msar.drop_table(tab_id => rel_id, cascade_ => false, if_exists => false); RETURN NEXT hasnt_table('dropme', 'Drops table'); @@ -68,6 +71,7 @@ CREATE OR REPLACE FUNCTION test_drop_table_oid_if_exists() RETURNS SETOF TEXT AS DECLARE rel_id oid; BEGIN + PERFORM __setup_drop_tables(); rel_id := 'dropme'::regclass::oid; PERFORM msar.drop_table(tab_id => rel_id, cascade_ => false, if_exists => true); RETURN NEXT hasnt_table('dropme', 'Drops table with IF EXISTS'); @@ -79,6 +83,7 @@ CREATE OR REPLACE FUNCTION test_drop_table_oid_restricted_fkey() RETURNS SETOF T DECLARE rel_id oid; BEGIN + PERFORM __setup_drop_tables(); rel_id := 'dropme'::regclass::oid; CREATE TABLE dependent (id SERIAL PRIMARY KEY, col1 integer REFERENCES dropme); @@ -96,6 +101,7 @@ CREATE OR REPLACE FUNCTION test_drop_table_oid_cascade_fkey() RETURNS SETOF TEXT DECLARE rel_id oid; BEGIN + PERFORM __setup_drop_tables(); rel_id := 'dropme'::regclass::oid; CREATE TABLE dependent (id SERIAL PRIMARY KEY, col1 integer REFERENCES dropme); @@ -107,6 +113,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_table_name() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_drop_tables(); PERFORM msar.drop_table( sch_name => 'public', tab_name => 'dropme', @@ -120,6 +127,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_table_name_missing_if_exists() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_drop_tables(); PERFORM msar.drop_table( sch_name => 'public', tab_name => 'dropmenew', @@ -201,7 +209,7 @@ $f$ LANGUAGE plpgsql; -- msar.add_columns -------------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION setup_add_columns() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_add_columns() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE add_col_testable (id serial primary key, col1 integer, col2 varchar); END; @@ -215,6 +223,7 @@ DECLARE {"name": "tcol", "type": {"name": "text"}, "not_null": true, "default": "my super default"} ]$j$; BEGIN + PERFORM __setup_add_columns(); RETURN NEXT is( msar.add_columns('add_col_testable'::regclass::oid, col_create_arr), '{4}'::smallint[] ); @@ -233,6 +242,7 @@ default value. The name should be "Column ", where is the attnum of the a DECLARE col_create_arr jsonb := '[{"type": {"name": "text"}}]'; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); RETURN NEXT col_is_null('add_col_testable', 'Column 4'); RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'text'); @@ -245,10 +255,12 @@ CREATE OR REPLACE FUNCTION test_add_columns_comment() RETURNS SETOF TEXT AS $f$ DECLARE col_name text := 'tcol'; description text := 'Some; comment with a semicolon'; - tab_id integer := 'add_col_testable'::regclass::oid; + tab_id integer; col_id integer; col_create_arr jsonb; BEGIN + PERFORM __setup_add_columns(); + tab_id := 'add_col_testable'::regclass::oid; col_create_arr := format('[{"name": "%s", "description": "%s"}]', col_name, description); PERFORM msar.add_columns(tab_id, col_create_arr); col_id := msar.get_attnum(tab_id, col_name); @@ -268,6 +280,7 @@ default value. The name should be "Column ", where is the attnum of the a DECLARE col_create_arr jsonb := '[{"type": {"name": "text"}}, {"type": {"name": "numeric"}}]'; BEGIN + PERFORM __setup_add_columns(); RETURN NEXT is( msar.add_columns('add_col_testable'::regclass::oid, col_create_arr), '{4, 5}'::smallint[] ); @@ -281,6 +294,7 @@ CREATE OR REPLACE FUNCTION test_add_columns_numeric_def() RETURNS SETOF TEXT AS DECLARE col_create_arr jsonb := '[{"type": {"name": "numeric"}, "default": 3.14159}]'; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'numeric'); RETURN NEXT col_default_is('add_col_testable', 'Column 4', 3.14159); @@ -292,6 +306,7 @@ CREATE OR REPLACE FUNCTION test_add_columns_numeric_prec() RETURNS SETOF TEXT AS DECLARE col_create_arr jsonb := '[{"type": {"name": "numeric", "options": {"precision": 3}}}]'; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'numeric(3,0)'); END; @@ -304,6 +319,7 @@ DECLARE {"type": {"name": "numeric", "options": {"precision": 3, "scale": 2}}} ]$j$; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'numeric(3,2)'); END; @@ -314,6 +330,7 @@ CREATE OR REPLACE FUNCTION test_add_columns_caps_numeric() RETURNS SETOF TEXT AS DECLARE col_create_arr jsonb := '[{"type": {"name": "NUMERIC"}}]'; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'numeric'); END; @@ -324,6 +341,7 @@ CREATE OR REPLACE FUNCTION test_add_columns_varchar_length() RETURNS SETOF TEXT DECLARE col_create_arr jsonb := '[{"type": {"name": "varchar", "options": {"length": 128}}}]'; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'character varying(128)'); END; @@ -333,45 +351,48 @@ CREATE OR REPLACE FUNCTION test_add_columns_interval_precision() RETURNS SETOF T DECLARE col_create_arr jsonb := '[{"type": {"name": "interval", "options": {"precision": 6}}}]'; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'interval(6)'); END; $f$ LANGUAGE plpgsql; --- upstream pgTAP bug: https://github.com/theory/pgtap/issues/315 --- CREATE OR REPLACE FUNCTION test_add_columns_interval_fields() RETURNS SETOF TEXT AS $f$ --- DECLARE --- col_create_arr jsonb := '[{"type": {"name": "interval", "options": {"fields": "year"}}}]'; --- BEGIN --- PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); --- RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'interval year'); --- END; --- $f$ LANGUAGE plpgsql; --- --- --- CREATE OR REPLACE FUNCTION test_add_columns_interval_fields_prec() RETURNS SETOF TEXT AS $f$ --- DECLARE --- col_create_arr jsonb := $j$ --- [{"type": {"name": "interval", "options": {"fields": "second", "precision": 3}}}] --- $j$; --- BEGIN --- PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); --- RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'interval second(3)'); --- END; --- $f$ LANGUAGE plpgsql; --- --- --- CREATE OR REPLACE FUNCTION test_add_columns_timestamp_prec() RETURNS SETOF TEXT AS $f$ --- DECLARE --- col_create_arr jsonb := $j$ --- [{"type": {"name": "timestamp", "options": {"precision": 3}}}] --- $j$; --- BEGIN --- PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); --- RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'timestamp(3) without time zone'); --- END; --- $f$ LANGUAGE plpgsql; +CREATE OR REPLACE FUNCTION test_add_columns_interval_fields() RETURNS SETOF TEXT AS $f$ +DECLARE + col_create_arr jsonb := '[{"type": {"name": "interval", "options": {"fields": "year"}}}]'; +BEGIN + PERFORM __setup_add_columns(); + PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); + RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'interval year'); +END; +$f$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_add_columns_interval_fields_prec() RETURNS SETOF TEXT AS $f$ +DECLARE + col_create_arr jsonb := $j$ + [{"type": {"name": "interval", "options": {"fields": "second", "precision": 3}}}] + $j$; +BEGIN + PERFORM __setup_add_columns(); + PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); + RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'interval second(3)'); +END; +$f$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_add_columns_timestamp_prec() RETURNS SETOF TEXT AS $f$ +DECLARE + col_create_arr jsonb := $j$ + [{"type": {"name": "timestamp", "options": {"precision": 3}}}] + $j$; +BEGIN + PERFORM __setup_add_columns(); + PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr); + RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'timestamp(3) without time zone'); +END; +$f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_columns_timestamp_raw_default() RETURNS SETOF TEXT AS $f$ @@ -381,6 +402,7 @@ This test will fail if the default is being sanitized, but will succeed if it's DECLARE col_create_arr jsonb := '[{"type": {"name": "timestamp"}, "default": "now()::timestamp"}]'; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr, raw_default => true); RETURN NEXT col_type_is('add_col_testable', 'Column 4', 'timestamp without time zone'); RETURN NEXT col_default_is( @@ -401,6 +423,7 @@ DECLARE [{"type": {"name": "text"}, "default": "null; drop table add_col_testable"}] $j$; BEGIN + PERFORM __setup_add_columns(); PERFORM msar.add_columns('add_col_testable'::regclass::oid, col_create_arr, raw_default => false); RETURN NEXT has_table('add_col_testable'); END; @@ -409,6 +432,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_columns_errors() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_add_columns(); RETURN NEXT throws_ok( format( 'SELECT msar.add_columns(tab_id => %s, col_defs => ''%s'');', @@ -436,7 +460,7 @@ $f$ LANGUAGE plpgsql; -- msar.copy_column -------------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION setup_copy_column() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_copy_column() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE copy_coltest ( id SERIAL PRIMARY KEY, @@ -460,6 +484,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_copies_unique() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 2::smallint, 'col1 supercopy', true, true ); @@ -481,6 +506,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_copies_unique_and_nnull() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 3::smallint, null, true, true ); @@ -502,6 +528,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_false_copy_data_and_con() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 3::smallint, null, false, false ); @@ -518,6 +545,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_num_options_static_default() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 4::smallint, null, true, false ); @@ -534,6 +562,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_nullable_dynamic_default() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 5::smallint, null, true, false ); @@ -546,6 +575,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_non_null_dynamic_default() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 6::smallint, null, true, true ); @@ -556,19 +586,20 @@ END; $f$ LANGUAGE plpgsql; --- upstream pgTAP bug: https://github.com/theory/pgtap/issues/315 --- CREATE OR REPLACE FUNCTION test_copy_column_interval_notation() RETURNS SETOF TEXT AS $f$ --- BEGIN --- PERFORM msar.copy_column( --- 'copy_coltest'::regclass::oid, 7::smallint, null, false, false --- ); --- RETURN NEXT col_type_is('copy_coltest', 'col6 1', 'interval second(3)'); --- END; --- $f$ LANGUAGE plpgsql; +CREATE OR REPLACE FUNCTION test_copy_column_interval_notation() RETURNS SETOF TEXT AS $f$ +BEGIN + PERFORM __setup_copy_column(); + PERFORM msar.copy_column( + 'copy_coltest'::regclass::oid, 7::smallint, null, false, false + ); + RETURN NEXT col_type_is('copy_coltest', 'col6 1', 'interval second(3)'); +END; +$f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_space_name() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 8::smallint, null, false, false ); @@ -579,6 +610,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_pkey() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 1::smallint, null, true, true ); @@ -595,6 +627,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_copy_column_increment_name() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_copy_column(); PERFORM msar.copy_column( 'copy_coltest'::regclass::oid, 2::smallint, null, true, true ); @@ -608,7 +641,7 @@ $f$ LANGUAGE plpgsql; -- msar.add_constraints ---------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION setup_add_pkey() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_add_pkey() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE add_pkeytest (col1 serial, col2 serial, col3 text); INSERT INTO add_pkeytest (col1, col2, col3) VALUES @@ -631,6 +664,7 @@ DECLARE created_name text; deferrable_ boolean; BEGIN + PERFORM __setup_add_pkey(); PERFORM msar.add_constraints('add_pkeytest'::regclass::oid, con_create_arr); RETURN NEXT col_is_pk('add_pkeytest', 'col1'); created_name := conname FROM pg_constraint @@ -647,6 +681,7 @@ DECLARE con_create_arr jsonb := '[{"type": "p", "columns": [1]}]'; created_name text; BEGIN + PERFORM __setup_add_pkey(); PERFORM msar.add_constraints('add_pkeytest'::regclass::oid, con_create_arr); RETURN NEXT col_is_pk('add_pkeytest', 'col1'); created_name := conname FROM pg_constraint @@ -661,6 +696,7 @@ DECLARE con_create_arr jsonb := '[{"type": "p", "columns": [1, 2]}]'; created_name text; BEGIN + PERFORM __setup_add_pkey(); PERFORM msar.add_constraints('add_pkeytest'::regclass::oid, con_create_arr); RETURN NEXT col_is_pk('add_pkeytest', ARRAY['col1', 'col2']); created_name := conname FROM pg_constraint @@ -674,6 +710,7 @@ CREATE OR REPLACE FUNCTION test_add_constraint_pkey_tab_name_singlecol() RETURNS DECLARE con_create_arr jsonb := '[{"type": "p", "columns": [1]}]'; BEGIN + PERFORM __setup_add_pkey(); PERFORM msar.add_constraints('public', 'add_pkeytest', con_create_arr); RETURN NEXT col_is_pk('add_pkeytest', 'col1'); END; @@ -684,6 +721,7 @@ CREATE OR REPLACE FUNCTION test_add_constraint_pkey_col_name_singlecol() RETURNS DECLARE con_create_arr jsonb := '[{"type": "p", "columns": ["col1"]}]'; BEGIN + PERFORM __setup_add_pkey(); PERFORM msar.add_constraints('add_pkeytest'::regclass::oid, con_create_arr); RETURN NEXT col_is_pk('add_pkeytest', 'col1'); END; @@ -694,6 +732,7 @@ CREATE OR REPLACE FUNCTION test_add_constraint_pkey_col_name_multicol() RETURNS DECLARE con_create_arr jsonb := '[{"type": "p", "columns": ["col1", "col2"]}]'; BEGIN + PERFORM __setup_add_pkey(); PERFORM msar.add_constraints('add_pkeytest'::regclass::oid, con_create_arr); RETURN NEXT col_is_pk('add_pkeytest', ARRAY['col1', 'col2']); END; @@ -704,6 +743,7 @@ CREATE OR REPLACE FUNCTION test_add_constraint_pkey_col_mix_multicol() RETURNS S DECLARE con_create_arr jsonb := '[{"type": "p", "columns": [1, "col2"]}]'; BEGIN + PERFORM __setup_add_pkey(); PERFORM msar.add_constraints('add_pkeytest'::regclass::oid, con_create_arr); RETURN NEXT col_is_pk('add_pkeytest', ARRAY['col1', 'col2']); END; @@ -954,6 +994,7 @@ CREATE OR REPLACE FUNCTION test_add_constraint_errors() RETURNS SETOF TEXT AS $f DECLARE con_create_arr jsonb := '[{"type": "p", "columns": [7]}]'::jsonb; BEGIN + PERFORM __setup_add_pkey(); RETURN NEXT throws_ok( format( 'SELECT msar.add_constraints(%s, ''%s'');', From fd3077e40e03d9a718fa14f88f04b08f01627104 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 30 Apr 2024 13:36:10 +0800 Subject: [PATCH 11/28] move remaining setup functions into callers for efficiency --- db/sql/test_0_msar.sql | 88 +++++++++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 22 deletions(-) diff --git a/db/sql/test_0_msar.sql b/db/sql/test_0_msar.sql index c5cecb832c..a4683f3d1b 100644 --- a/db/sql/test_0_msar.sql +++ b/db/sql/test_0_msar.sql @@ -750,7 +750,7 @@ END; $f$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_add_fkey() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_add_fkey() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE add_fk_users (id serial primary key, fname TEXT, lname TEXT, phoneno TEXT); INSERT INTO add_fk_users (fname, lname, phoneno) VALUES @@ -776,6 +776,7 @@ CREATE OR REPLACE FUNCTION test_add_constraint_fkey_id_fullspec() RETURNS SETOF DECLARE con_create_arr jsonb; BEGIN + PERFORM __setup_add_fkey(); con_create_arr := format( $j$[ { @@ -809,6 +810,7 @@ CREATE OR REPLACE FUNCTION fkey_options_eq("char", "char", "char") RETURNS TEXT DECLARE con_create_arr jsonb; BEGIN + PERFORM __setup_add_fkey(); con_create_arr := format( $j$[ { @@ -847,7 +849,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_constraints_fkey_opts_aas() RETURNS SETOF TEXT AS $f$ BEGIN - RETURN NEXT fkey_options_eq('a', 'a', 's'); + PERFORM fkey_options_eq('a', 'a', 's'); RETURN NEXT fk_ok( 'public', 'add_fk_comments', 'user_id', 'public', 'add_fk_users', 'id' ); @@ -857,7 +859,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_constraints_fkey_opts_arf() RETURNS SETOF TEXT AS $f$ BEGIN - RETURN NEXT fkey_options_eq('a', 'r', 'f'); + PERFORM fkey_options_eq('a', 'r', 'f'); RETURN NEXT fk_ok( 'public', 'add_fk_comments', 'user_id', 'public', 'add_fk_users', 'id' ); @@ -867,7 +869,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_constraints_fkey_opts_rrf() RETURNS SETOF TEXT AS $f$ BEGIN - RETURN NEXT fkey_options_eq('r', 'r', 'f'); + PERFORM fkey_options_eq('r', 'r', 'f'); RETURN NEXT fk_ok( 'public', 'add_fk_comments', 'user_id', 'public', 'add_fk_users', 'id' ); @@ -877,7 +879,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_constraints_fkey_opts_rrf() RETURNS SETOF TEXT AS $f$ BEGIN - RETURN NEXT fkey_options_eq('r', 'r', 'f'); + PERFORM fkey_options_eq('r', 'r', 'f'); RETURN NEXT fk_ok( 'public', 'add_fk_comments', 'user_id', 'public', 'add_fk_users', 'id' ); @@ -887,7 +889,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_constraints_fkey_opts_ccf() RETURNS SETOF TEXT AS $f$ BEGIN - RETURN NEXT fkey_options_eq('c', 'c', 'f'); + PERFORM fkey_options_eq('c', 'c', 'f'); RETURN NEXT fk_ok( 'public', 'add_fk_comments', 'user_id', 'public', 'add_fk_users', 'id' ); @@ -897,7 +899,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_constraints_fkey_opts_nnf() RETURNS SETOF TEXT AS $f$ BEGIN - RETURN NEXT fkey_options_eq('n', 'n', 'f'); + PERFORM fkey_options_eq('n', 'n', 'f'); RETURN NEXT fk_ok( 'public', 'add_fk_comments', 'user_id', 'public', 'add_fk_users', 'id' ); @@ -907,7 +909,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_constraints_fkey_opts_ddf() RETURNS SETOF TEXT AS $f$ BEGIN - RETURN NEXT fkey_options_eq('d', 'd', 'f'); + PERFORM fkey_options_eq('d', 'd', 'f'); RETURN NEXT fk_ok( 'public', 'add_fk_comments', 'user_id', 'public', 'add_fk_users', 'id' ); @@ -915,7 +917,7 @@ END; $f$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_add_unique() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_add_unique() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE add_unique_con (id serial primary key, col1 integer, col2 integer, col3 integer); INSERT INTO add_unique_con (col1, col2, col3) VALUES @@ -930,6 +932,7 @@ CREATE OR REPLACE FUNCTION test_add_constraints_unique_single() RETURNS SETOF T DECLARE con_create_arr jsonb := '[{"name": "myuniqcons", "type": "u", "columns": [2]}]'; BEGIN + PERFORM __setup_add_unique(); PERFORM msar.add_constraints('add_unique_con'::regclass::oid, con_create_arr); RETURN NEXT col_is_unique('add_unique_con', ARRAY['col1']); END; @@ -940,6 +943,7 @@ CREATE OR REPLACE FUNCTION test_add_constraints_unique_multicol() RETURNS SETOF DECLARE con_create_arr jsonb := '[{"name": "myuniqcons", "type": "u", "columns": [2, 3]}]'; BEGIN + PERFORM __setup_add_unique(); PERFORM msar.add_constraints('add_unique_con'::regclass::oid, con_create_arr); RETURN NEXT col_is_unique('add_unique_con', ARRAY['col1', 'col2']); END; @@ -951,6 +955,7 @@ DECLARE con_create_arr jsonb := '[{"name": "myuniqcons", "type": "u", "columns": [2]}]'; con_create_arr2 jsonb := '[{"name": "myuniqcons", "type": "u", "columns": [3]}]'; BEGIN + PERFORM __setup_add_unique(); PERFORM msar.add_constraints('add_unique_con'::regclass::oid, con_create_arr); RETURN NEXT throws_ok( format( @@ -964,7 +969,7 @@ END; $f$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_copy_unique() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_copy_unique() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE copy_unique_con (id serial primary key, col1 integer, col2 integer, col3 integer, col4 integer); @@ -981,6 +986,7 @@ CREATE OR REPLACE FUNCTION test_copy_constraint() RETURNS SETOF TEXT AS $f$ DECLARE orig_oid oid; BEGIN + PERFORM __setup_copy_unique(); orig_oid := oid FROM pg_constraint WHERE conrelid='copy_unique_con'::regclass::oid AND conname='olduniqcon'; @@ -1039,7 +1045,7 @@ $f$ LANGUAGE plpgsql; -- msar.drop_constraint --------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION setup_drop_constraint() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_drop_constraint() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE category( id serial primary key, @@ -1059,6 +1065,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_constraint() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_drop_constraint(); PERFORM msar.drop_constraint( sch_name => 'public', tab_name => 'category', @@ -1082,6 +1089,7 @@ DECLARE uq_cat_oid oid; fk_cat_oid oid; BEGIN + PERFORM __setup_drop_constraint(); uq_cat_oid := oid FROM pg_constraint WHERE conname='uq_cat'; fk_cat_oid := oid FROM pg_constraint WHERE conname='fk_cat'; PERFORM msar.drop_constraint( @@ -1102,7 +1110,7 @@ $$ LANGUAGE plpgsql; -- msar.create_link ------------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION setup_link_tables() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_link_tables() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE actors (id SERIAL PRIMARY KEY, actor_name text); INSERT INTO actors(actor_name) VALUES @@ -1124,6 +1132,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_create_many_to_one_link() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_link_tables(); PERFORM msar.create_many_to_one_link( frel_id => 'actors'::regclass::oid, rel_id => 'movies'::regclass::oid, @@ -1138,6 +1147,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_create_one_to_one_link() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_link_tables(); PERFORM msar.create_many_to_one_link( frel_id => 'actors'::regclass::oid, rel_id => 'movies'::regclass::oid, @@ -1154,6 +1164,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_create_many_to_many_link() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_link_tables(); PERFORM msar.create_many_to_many_link( sch_id => 'public'::regnamespace::oid, tab_name => 'movies_actors', @@ -1184,7 +1195,7 @@ END; $$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_drop_schema() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_drop_schema() RETURNS SETOF TEXT AS $$ BEGIN CREATE SCHEMA drop_test_schema; END; @@ -1193,6 +1204,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_schema_if_exists_false() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_drop_schema(); PERFORM msar.drop_schema( sch_name => 'drop_test_schema', cascade_ => false, @@ -1217,6 +1229,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_schema_if_exists_true() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_drop_schema(); PERFORM msar.drop_schema( sch_name => 'drop_test_schema', cascade_ => false, @@ -1239,6 +1252,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_schema_using_oid() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_drop_schema(); PERFORM msar.drop_schema( sch_id => 'drop_test_schema'::regnamespace::oid, cascade_ => false, @@ -1249,7 +1263,7 @@ END; $$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_schema_with_dependent_obj() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_schema_with_dependent_obj() RETURNS SETOF TEXT AS $$ BEGIN CREATE SCHEMA schema1; CREATE TABLE schema1.actors ( @@ -1262,6 +1276,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_schema_cascade() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_schema_with_dependent_obj(); PERFORM msar.drop_schema( sch_name => 'schema1', cascade_ => true, @@ -1274,6 +1289,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_drop_schema_restricted() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_schema_with_dependent_obj(); RETURN NEXT throws_ok( format( 'SELECT msar.drop_schema( @@ -1290,7 +1306,7 @@ END; $$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_alter_schema() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_alter_schema() RETURNS SETOF TEXT AS $$ BEGIN CREATE SCHEMA alter_me; END; @@ -1299,6 +1315,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_rename_schema() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_alter_schema(); PERFORM msar.rename_schema( old_sch_name => 'alter_me', new_sch_name => 'altered' @@ -1311,6 +1328,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_rename_schema_using_oid() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_alter_schema(); PERFORM msar.rename_schema( sch_id => 'alter_me'::regnamespace::oid, new_sch_name => 'altered' @@ -1323,6 +1341,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_comment_on_schema() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_alter_schema(); PERFORM msar.comment_on_schema( sch_name => 'alter_me', comment_ => 'test comment' @@ -1334,7 +1353,7 @@ $$ LANGUAGE plpgsql; -- msar.alter_table -CREATE OR REPLACE FUNCTION setup_alter_table() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_alter_table() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE alter_this_table(id INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, col1 TEXT); END; @@ -1343,6 +1362,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_rename_table() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_alter_table(); PERFORM msar.rename_table( sch_name =>'public', old_tab_name => 'alter_this_table', @@ -1356,6 +1376,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_rename_table_using_oid() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_alter_table(); PERFORM msar.rename_table( tab_id => 'alter_this_table'::regclass::oid, new_tab_name => 'renamed_table' @@ -1368,6 +1389,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_comment_on_table() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_alter_table(); PERFORM msar.comment_on_table( sch_name =>'public', tab_name => 'alter_this_table', @@ -1380,6 +1402,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_comment_on_table_using_oid() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_alter_table(); PERFORM msar.comment_on_table( tab_id => 'alter_this_table'::regclass::oid, comment_ => 'This is a comment!' @@ -1391,7 +1414,7 @@ $$ LANGUAGE plpgsql; -- msar.add_mathesar_table -CREATE OR REPLACE FUNCTION setup_create_table() RETURNS SETOF TEXT AS $f$ +CREATE OR REPLACE FUNCTION __setup_create_table() RETURNS SETOF TEXT AS $f$ BEGIN CREATE SCHEMA tab_create_schema; END; @@ -1400,6 +1423,7 @@ $f$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_add_mathesar_table_minimal_id_col() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_create_table(); PERFORM msar.add_mathesar_table( 'tab_create_schema'::regnamespace::oid, 'anewtable', null, null, null ); @@ -1421,6 +1445,7 @@ CREATE OR REPLACE FUNCTION test_add_mathesar_table_badname() RETURNS SETOF TEXT DECLARE badname text := $b$M"new"'dsf' \t"$b$; BEGIN + PERFORM __setup_create_table(); PERFORM msar.add_mathesar_table( 'tab_create_schema'::regnamespace::oid, badname, null, null, null ); @@ -1437,6 +1462,7 @@ DECLARE {"type": {"name": "varchar", "options": {"length": 128}}} ]$j$; BEGIN + PERFORM __setup_create_table(); PERFORM msar.add_mathesar_table( 'tab_create_schema'::regnamespace::oid, 'cols_table', @@ -1460,6 +1486,7 @@ CREATE OR REPLACE FUNCTION test_add_mathesar_table_comment() RETURNS SETOF TEXT DECLARE comment_ text := $c$my "Super;";'; DROP SCHEMA tab_create_schema;'$c$; BEGIN + PERFORM __setup_create_table(); PERFORM msar.add_mathesar_table( 'tab_create_schema'::regnamespace::oid, 'cols_table', null, null, comment_ ); @@ -1475,7 +1502,7 @@ END; $f$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_column_alter() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_column_alter() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE col_alters ( id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, @@ -1501,6 +1528,7 @@ development, and runs quickly. DECLARE tab_id oid; BEGIN + PERFORM __setup_column_alter(); tab_id := 'col_alters'::regclass::oid; RETURN NEXT is(msar.process_col_alter_jsonb(tab_id, '[{"attnum": 2}]'), null); RETURN NEXT is(msar.process_col_alter_jsonb(tab_id, '[{"attnum": 2, "name": "blah"}]'), null); @@ -1513,6 +1541,7 @@ CREATE OR REPLACE FUNCTION test_alter_columns_single_name() RETURNS SETOF TEXT A DECLARE col_alters_jsonb jsonb := '[{"attnum": 2, "name": "blah"}]'; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[2]); RETURN NEXT columns_are( 'col_alters', @@ -1529,6 +1558,7 @@ DECLARE {"attnum": 4, "name": "nospace"} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[2, 4]); RETURN NEXT columns_are( 'col_alters', @@ -1546,6 +1576,7 @@ DECLARE {"attnum": 4, "type": {"name": "integer"}} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[2, 3, 4]); RETURN NEXT col_type_is('col_alters', 'col1', 'character varying(48)'); RETURN NEXT col_type_is('col_alters', 'col2', 'integer'); @@ -1561,6 +1592,7 @@ DECLARE {"attnum": 5, "type": {"options": {"precision": 4}}} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[5]); RETURN NEXT col_type_is('col_alters', 'col_opts', 'numeric(4,0)'); END; @@ -1574,6 +1606,7 @@ DECLARE {"attnum": 5, "delete": true} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[2, 5]); RETURN NEXT columns_are('col_alters', ARRAY['id', 'col2', 'Col sp', 'coltim']); END; @@ -1587,6 +1620,7 @@ DECLARE {"attnum": 5, "not_null": true} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[2, 5]); RETURN NEXT col_is_null('col_alters', 'col1'); RETURN NEXT col_not_null('col_alters', 'col_opts'); @@ -1601,6 +1635,7 @@ DECLARE {"attnum": 6, "type": {"name": "date"}} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[3, 6]); RETURN NEXT col_default_is('col_alters', 'col2', '5'); RETURN NEXT col_default_is('col_alters', 'coltim', '(now())::date'); @@ -1615,6 +1650,7 @@ DECLARE {"attnum": 6, "type": {"name": "date"}, "default": null} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[3, 6]); RETURN NEXT col_hasnt_default('col_alters', 'col2'); RETURN NEXT col_hasnt_default('col_alters', 'coltim'); @@ -1631,6 +1667,7 @@ DECLARE {"attnum": 6, "type": {"name": "text"}, "default": "test12"} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is( msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[2, 3, 5, 6] @@ -1659,6 +1696,7 @@ DECLARE {"attnum": 6, "name": "timecol", "not_null": true} ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is( msar.alter_columns('col_alters'::regclass::oid, col_alters_jsonb), ARRAY[2, 3, 4, 5, 6] ); @@ -1722,6 +1760,7 @@ DECLARE } ]$j$; BEGIN + PERFORM __setup_column_alter(); RETURN NEXT is(msar.col_description('col_alters'::regclass::oid, 2), NULL); PERFORM msar.alter_columns('col_alters'::regclass::oid, change1); RETURN NEXT is(msar.col_description('col_alters'::regclass::oid, 2), 'change1col2description'); @@ -1737,7 +1776,7 @@ END; $$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_roster() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_roster() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE "Roster" ( id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, @@ -2006,6 +2045,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_extract_columns_data() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_roster(); CREATE TABLE roster_snapshot AS SELECT * FROM "Roster" ORDER BY id; PERFORM msar.extract_columns_from_table('"Roster"'::regclass::oid, ARRAY[3, 4], 'Teachers', null); RETURN NEXT columns_are('Teachers', ARRAY['id', 'Teacher', 'Teacher Email']); @@ -2039,7 +2079,7 @@ END; $f$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_extract_fkey_cols() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_extract_fkey_cols() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE "Referent" ( id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, @@ -2059,6 +2099,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_extract_columns_keeps_fkey() RETURNS SETOF TEXT AS $f$ BEGIN + PERFORM __setup_extract_fkey_cols(); PERFORM msar.extract_columns_from_table( '"Referrer"'::regclass::oid, ARRAY[3, 5], 'Classes', 'Class' ); @@ -2071,7 +2112,7 @@ END; $f$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_dynamic_defaults() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_dynamic_defaults() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE defaults_test ( id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, @@ -2089,6 +2130,7 @@ CREATE OR REPLACE FUNCTION test_is_possibly_dynamic() RETURNS SETOF TEXT AS $$ DECLARE tab_id oid; BEGIN + PERFORM __setup_dynamic_defaults(); tab_id := 'defaults_test'::regclass::oid; RETURN NEXT is(msar.is_default_possibly_dynamic(tab_id, 1), true); RETURN NEXT is(msar.is_default_possibly_dynamic(tab_id, 2), false); @@ -2122,7 +2164,7 @@ END; $$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION setup_interval_fields() RETURNS SETOF TEXT AS $$ +CREATE OR REPLACE FUNCTION __setup_manytypes() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE manytypes ( id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, @@ -2175,6 +2217,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_get_interval_fields() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_manytypes(); RETURN NEXT results_eq( $h$ SELECT msar.get_interval_fields(atttypmod) @@ -2218,6 +2261,7 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION test_get_type_options() RETURNS SETOF TEXT AS $$ BEGIN + PERFORM __setup_manytypes(); RETURN NEXT is(msar.get_type_options(atttypid, atttypmod, attndims), NULL) FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='id'; RETURN NEXT is( From d44c0b8a760886262e85b0e5e6f5e72aa7399589 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 30 Apr 2024 15:10:19 +0800 Subject: [PATCH 12/28] add get_column_info test --- db/sql/test_0_msar.sql | 213 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 212 insertions(+), 1 deletion(-) diff --git a/db/sql/test_0_msar.sql b/db/sql/test_0_msar.sql index a4683f3d1b..40c2d7c5ab 100644 --- a/db/sql/test_0_msar.sql +++ b/db/sql/test_0_msar.sql @@ -2164,6 +2164,8 @@ END; $$ LANGUAGE plpgsql; +-- msar.get_column_info (and related) -------------------------------------------------------------- + CREATE OR REPLACE FUNCTION __setup_manytypes() RETURNS SETOF TEXT AS $$ BEGIN CREATE TABLE manytypes ( @@ -2209,7 +2211,13 @@ CREATE TABLE manytypes ( cha_1 character, cha_20 character(20), var_16_arr varchar(16)[], - cha_20_arr character(20)[][] + cha_20_arr character(20)[][], + bit_8 bit(8), + vbt_8 varbit(8), + tim_2 time(2), + ttz_3 timetz(3), + tsp_4 timestamp(4), + tsz_5 timestamptz(5) ); END; $$ LANGUAGE plpgsql; @@ -2354,5 +2362,208 @@ BEGIN '{"length": 20, "item_type": "character"}'::jsonb ) FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='cha_20_arr'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 8}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='bit_8'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 8}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='vbt_8'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 2}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='tim_2'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 3}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='ttz_3'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 4}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='tsp_4'; + RETURN NEXT is( + msar.get_type_options(atttypid, atttypmod, attndims), + '{"precision": 5}'::jsonb + ) + FROM pg_attribute WHERE attrelid='manytypes'::regclass AND attname='tsz_5'; +END; +$$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION __setup_cast_functions() RETURNS SETOF TEXT AS $$ +BEGIN + CREATE SCHEMA mathesar_types; + CREATE FUNCTION mathesar_types.cast_to_numeric(text) RETURNS numeric AS 'SELECT 5' LANGUAGE SQL; + CREATE FUNCTION mathesar_types.cast_to_text(text) RETURNS text AS 'SELECT ''5''' LANGUAGE SQL; +END; +$$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_get_valid_target_type_strings() RETURNS SETOF TEXT AS $$ +BEGIN + PERFORM __setup_cast_functions(); + RETURN NEXT is(msar.get_valid_target_type_strings('text'), '["numeric", "text"]'::jsonb); + RETURN NEXT is( + msar.get_valid_target_type_strings('text'::regtype::oid), '["numeric", "text"]'::jsonb + ); + RETURN NEXT is(msar.get_valid_target_type_strings('interval'), NULL); +END; +$$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_has_dependents() RETURNS SETOF TEXT AS $$ +BEGIN + PERFORM __setup_extract_fkey_cols(); + RETURN NEXT is(msar.has_dependents('"Referent"'::regclass::oid, 1::smallint), true); + RETURN NEXT is(msar.has_dependents('"Referent"'::regclass::oid, 2::smallint), false); + RETURN NEXT is(msar.has_dependents('"Referrer"'::regclass::oid, 1::smallint), false); +END; +$$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION __setup_get_column_info() RETURNS SETOF TEXT AS $$ +BEGIN + PERFORM __setup_cast_functions(); + CREATE TABLE column_variety ( + id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, + num_plain numeric NOT NULL, + var_128 varchar(128), + txt text DEFAULT 'abc', + tst timestamp DEFAULT NOW(), + int_arr integer[4][3], + num_opt_arr numeric(15, 10)[] + ); + COMMENT ON COLUMN column_variety.txt IS 'A super comment ;'; + CREATE TABLE needs_cv ( + id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, + cv_id integer REFERENCES column_variety(id) + ); +END; +$$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_get_column_info() RETURNS SETOF TEXT AS $$ +BEGIN + PERFORM __setup_get_column_info(); + RETURN NEXT is( + msar.get_column_info('column_variety'), + $j$[ + { + "id": 1, + "name": "id", + "type": "integer", + "default": { + "value": "identity", + "is_dynamic": true + }, + "nullable": false, + "description": null, + "primary_key": true, + "type_options": null, + "has_dependents": true, + "valid_target_types": null + }, + { + "id": 2, + "name": "num_plain", + "type": "numeric", + "default": null, + "nullable": false, + "description": null, + "primary_key": false, + "type_options": { + "scale": null, + "precision": null + }, + "has_dependents": false, + "valid_target_types": null + }, + { + "id": 3, + "name": "var_128", + "type": "character varying", + "default": null, + "nullable": true, + "description": null, + "primary_key": false, + "type_options": { + "length": 128 + }, + "has_dependents": false, + "valid_target_types": null + }, + { + "id": 4, + "name": "txt", + "type": "text", + "default": { + "value": "'abc'::text", + "is_dynamic": false + }, + "nullable": true, + "description": "A super comment ;", + "primary_key": false, + "type_options": null, + "has_dependents": false, + "valid_target_types": [ + "numeric", + "text" + ] + }, + { + "id": 5, + "name": "tst", + "type": "timestamp without time zone", + "default": { + "value": "now()", + "is_dynamic": true + }, + "nullable": true, + "description": null, + "primary_key": false, + "type_options": { + "precision": null + }, + "has_dependents": false, + "valid_target_types": null + }, + { + "id": 6, + "name": "int_arr", + "type": "_array", + "default": null, + "nullable": true, + "description": null, + "primary_key": false, + "type_options": { + "item_type": "integer" + }, + "has_dependents": false, + "valid_target_types": null + }, + { + "id": 7, + "name": "num_opt_arr", + "type": "_array", + "default": null, + "nullable": true, + "description": null, + "primary_key": false, + "type_options": { + "scale": 10, + "item_type": "numeric", + "precision": 15 + }, + "has_dependents": false, + "valid_target_types": null + } + ]$j$::jsonb + ); END; $$ LANGUAGE plpgsql; From 31829ae3991842874d0969bd2b6ca398e6ea0c80 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Thu, 2 May 2024 16:13:33 +0800 Subject: [PATCH 13/28] add column info getter in python layer --- db/columns/operations/select.py | 39 ++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/db/columns/operations/select.py b/db/columns/operations/select.py index d1b22aba6d..8426090dee 100644 --- a/db/columns/operations/select.py +++ b/db/columns/operations/select.py @@ -3,11 +3,48 @@ from sqlalchemy import and_, asc, cast, select, text, exists, Identity from db.columns.exceptions import DynamicDefaultWarning -from db.connection import execute_msar_func_with_engine +from db.connection import execute_msar_func_with_engine, exec_msar_func from db.tables.operations.select import reflect_table_from_oid from db.utils import execute_statement, get_pg_catalog_table +def get_column_info_for_table(table, conn): + """ + Return a list of dictionaries desrcibing the columns of the table. + + The `table` can be given as either a "qualified name", or an OID. + The OID is the preferred identifier, since it's much more robust. + + The returned list contains dictionaries of the following form: + + { + "id": , + "name": , + "type": , + "type_options": { + "precision": , + "scale": , + "fields": , + "length": , + "item_type": , + }, + "nullable": , + "primary_key": , + "valid_target_types": [, , ..., ] + "default": {"value": , "is_dynamic": }, + "has_dependents": , + "description": + } + + The fields of the "type_options" dictionary are all optional, + depending on the "type" value. + + Args: + table: The table for which we want column info. + """ + return exec_msar_func(conn, 'get_column_info', table).fetchone()[0] + + def get_column_description(oid, attnum, engine): cursor = execute_msar_func_with_engine(engine, 'col_description', oid, attnum) row = cursor.fetchone() From 993ee06e6f844530fab771bc74b1df8b74048bc1 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Thu, 2 May 2024 16:49:24 +0800 Subject: [PATCH 14/28] wire column info getter up to RPC endpoint --- config/settings/common_settings.py | 3 ++- mathesar/rpc/columns.py | 35 ++++++++++++++++++++++++++++++ mathesar/utils/columns.py | 12 ++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 mathesar/rpc/columns.py create mode 100644 mathesar/utils/columns.py diff --git a/config/settings/common_settings.py b/config/settings/common_settings.py index 251797af41..7b8ab40d58 100644 --- a/config/settings/common_settings.py +++ b/config/settings/common_settings.py @@ -65,7 +65,8 @@ def pipe_delim(pipe_string): ROOT_URLCONF = "config.urls" MODERNRPC_METHODS_MODULES = [ - 'mathesar.rpc.connections' + 'mathesar.rpc.connections', + 'mathesar.rpc.columns' ] TEMPLATES = [ diff --git a/mathesar/rpc/columns.py b/mathesar/rpc/columns.py new file mode 100644 index 0000000000..6db73658a7 --- /dev/null +++ b/mathesar/rpc/columns.py @@ -0,0 +1,35 @@ +""" +Classes and functions exposed to the RPC endpoint for managing table columns. +""" +from modernrpc.core import rpc_method +from modernrpc.auth.basic import http_basic_auth_login_required + +from db.columns.operations.select import get_column_info_for_table +from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions +from mathesar.rpc.utils import connect +from mathesar.utils.columns import get_display_options + +@rpc_method(name='columns.list') +@http_basic_auth_login_required +@handle_rpc_exceptions +def list(*, table_oid: int, database_id: int): + """ + List columns for a table, with information about each. + + Also return display options for each column, if they're defined. + + Args: + table_oid: Identity of the table in the user's database. + database_id: The Django model id of the database containing the table. + + Returns: + A list of column details, and a separate list of display options. + """ + with connect(database_id) as conn: + column_info = get_column_info_for_table(table_oid, conn) + attnums = [col['id'] for col in column_info] + display_options = get_display_options(table_oid, attnums) + return { + "column_info": column_info, + "display_options": display_options + } diff --git a/mathesar/utils/columns.py b/mathesar/utils/columns.py new file mode 100644 index 0000000000..d6fb4b0cad --- /dev/null +++ b/mathesar/utils/columns.py @@ -0,0 +1,12 @@ +from mathesar.models.base import Column + + +# This should be replaced once we have the ColumnMetadata model sorted out. +def get_display_options(table_oid, attnums): + return [ + {"id": c.attnum} | c.display_options + for c in Column.current_objects.filter( + table__oid=table_oid, attnum__in=attnums + ) + if c.display_options is not None + ] From 784276cf5ab911eb131b253552e18e4afc10e252 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 3 May 2024 15:31:53 +0800 Subject: [PATCH 15/28] remove valid_target_types as it should go somewhere global --- db/sql/0_msar.sql | 2 -- db/sql/test_0_msar.sql | 24 +++++++----------------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/db/sql/0_msar.sql b/db/sql/0_msar.sql index 45590aa3c4..8abac5ea2d 100644 --- a/db/sql/0_msar.sql +++ b/db/sql/0_msar.sql @@ -612,7 +612,6 @@ Each returned JSON object in the array will have the form: "type_options": , "nullable": , "primary_key": , - "valid_target_types": [, , ..., ] "default": {"value": , "is_dynamic": }, "has_dependents": , "description": @@ -631,7 +630,6 @@ SELECT jsonb_agg( 'type_options', msar.get_type_options(atttypid, atttypmod, attndims), 'nullable', NOT attnotnull, 'primary_key', COALESCE(pgi.indisprimary, false), - 'valid_target_types', msar.get_valid_target_type_strings(atttypid), 'default', nullif( jsonb_strip_nulls( diff --git a/db/sql/test_0_msar.sql b/db/sql/test_0_msar.sql index 40c2d7c5ab..f9d1b30317 100644 --- a/db/sql/test_0_msar.sql +++ b/db/sql/test_0_msar.sql @@ -2466,8 +2466,7 @@ BEGIN "description": null, "primary_key": true, "type_options": null, - "has_dependents": true, - "valid_target_types": null + "has_dependents": true }, { "id": 2, @@ -2481,8 +2480,7 @@ BEGIN "scale": null, "precision": null }, - "has_dependents": false, - "valid_target_types": null + "has_dependents": false }, { "id": 3, @@ -2495,8 +2493,7 @@ BEGIN "type_options": { "length": 128 }, - "has_dependents": false, - "valid_target_types": null + "has_dependents": false }, { "id": 4, @@ -2510,11 +2507,7 @@ BEGIN "description": "A super comment ;", "primary_key": false, "type_options": null, - "has_dependents": false, - "valid_target_types": [ - "numeric", - "text" - ] + "has_dependents": false }, { "id": 5, @@ -2530,8 +2523,7 @@ BEGIN "type_options": { "precision": null }, - "has_dependents": false, - "valid_target_types": null + "has_dependents": false }, { "id": 6, @@ -2544,8 +2536,7 @@ BEGIN "type_options": { "item_type": "integer" }, - "has_dependents": false, - "valid_target_types": null + "has_dependents": false }, { "id": 7, @@ -2560,8 +2551,7 @@ BEGIN "item_type": "numeric", "precision": 15 }, - "has_dependents": false, - "valid_target_types": null + "has_dependents": false } ]$j$::jsonb ); From 77ed3791383d620622d0b052f3fed60280bd2bda Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 3 May 2024 15:32:29 +0800 Subject: [PATCH 16/28] fix typo in docstring --- db/columns/operations/select.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/columns/operations/select.py b/db/columns/operations/select.py index 8426090dee..a79e63e1c8 100644 --- a/db/columns/operations/select.py +++ b/db/columns/operations/select.py @@ -10,7 +10,7 @@ def get_column_info_for_table(table, conn): """ - Return a list of dictionaries desrcibing the columns of the table. + Return a list of dictionaries describing the columns of the table. The `table` can be given as either a "qualified name", or an OID. The OID is the preferred identifier, since it's much more robust. From b2f15acc8ed9f61da2e95c638c0b5b11ee33c893 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 3 May 2024 15:34:01 +0800 Subject: [PATCH 17/28] add class to validate and describe column info response --- docs/docs/api/rpc.md | 8 ++++++++ mathesar/rpc/columns.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/docs/api/rpc.md b/docs/docs/api/rpc.md index 53ca6293d6..8f4230eea9 100644 --- a/docs/docs/api/rpc.md +++ b/docs/docs/api/rpc.md @@ -46,6 +46,14 @@ To use an RPC function: - add_from_scratch - DBModelReturn +--- + +::: mathesar.rpc.columns + options: + members: + - list + - ColumnInfo + ## Responses ### Success diff --git a/mathesar/rpc/columns.py b/mathesar/rpc/columns.py index 6db73658a7..f5a91fc5fc 100644 --- a/mathesar/rpc/columns.py +++ b/mathesar/rpc/columns.py @@ -1,6 +1,8 @@ """ Classes and functions exposed to the RPC endpoint for managing table columns. """ +from typing import TypedDict + from modernrpc.core import rpc_method from modernrpc.auth.basic import http_basic_auth_login_required @@ -9,6 +11,28 @@ from mathesar.rpc.utils import connect from mathesar.utils.columns import get_display_options + +class ColumnInfo(TypedDict): + """ + Information about a column. + + Attributes: + id (int): The `attnum` of the column in the table. + name (str): The name of the column. + type (str): The type of the column on the database. + type_options (dict): The options applied to the column type. + nullable (bool): Whether or not the column is nullable. + primary_key (bool): Whether the column is in the primary key. + default (dict): The default value and whether it's dynamic. + has_dependents (bool): Whether the column has dependent objects. + description (str): The description of the column. + """ + + @classmethod + def from_column_info_json(cls, col_info): + return cls(**col_info) + + @rpc_method(name='columns.list') @http_basic_auth_login_required @handle_rpc_exceptions @@ -25,8 +49,12 @@ def list(*, table_oid: int, database_id: int): Returns: A list of column details, and a separate list of display options. """ + # TODO Add user as arg for connect and get_display_options with connect(database_id) as conn: - column_info = get_column_info_for_table(table_oid, conn) + column_info = [ + ColumnInfo.from_column_info_json(col) + for col in get_column_info_for_table(table_oid, conn) + ] attnums = [col['id'] for col in column_info] display_options = get_display_options(table_oid, attnums) return { From 2c15a07b9a78ff1235defca8703dd48f1d923d42 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 3 May 2024 15:37:22 +0800 Subject: [PATCH 18/28] stub out permissions checks --- mathesar/models/users.py | 3 +++ mathesar/rpc/columns.py | 14 +++++++++----- mathesar/rpc/utils.py | 3 ++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/mathesar/models/users.py b/mathesar/models/users.py index 9e3fcbe86a..e1b77a8161 100644 --- a/mathesar/models/users.py +++ b/mathesar/models/users.py @@ -17,6 +17,9 @@ class User(AbstractUser): password_change_needed = models.BooleanField(default=False) display_language = models.CharField(max_length=30, blank=True, default='en') + def metadata_privileges(self, database_id): + return 'read write' + class Role(models.TextChoices): MANAGER = 'manager', 'Manager' diff --git a/mathesar/rpc/columns.py b/mathesar/rpc/columns.py index f5a91fc5fc..fa89cba65e 100644 --- a/mathesar/rpc/columns.py +++ b/mathesar/rpc/columns.py @@ -3,7 +3,7 @@ """ from typing import TypedDict -from modernrpc.core import rpc_method +from modernrpc.core import rpc_method, REQUEST_KEY from modernrpc.auth.basic import http_basic_auth_login_required from db.columns.operations.select import get_column_info_for_table @@ -36,7 +36,7 @@ def from_column_info_json(cls, col_info): @rpc_method(name='columns.list') @http_basic_auth_login_required @handle_rpc_exceptions -def list(*, table_oid: int, database_id: int): +def list(*, table_oid: int, database_id: int, **kwargs): """ List columns for a table, with information about each. @@ -50,13 +50,17 @@ def list(*, table_oid: int, database_id: int): A list of column details, and a separate list of display options. """ # TODO Add user as arg for connect and get_display_options - with connect(database_id) as conn: + request = kwargs.get(REQUEST_KEY) + with connect(database_id, request.user) as conn: column_info = [ ColumnInfo.from_column_info_json(col) for col in get_column_info_for_table(table_oid, conn) ] - attnums = [col['id'] for col in column_info] - display_options = get_display_options(table_oid, attnums) + if request.user.metadata_privileges(database_id) is not None: + attnums = [col['id'] for col in column_info] + display_options = get_display_options(table_oid, attnums) + else: + display_options = None return { "column_info": column_info, "display_options": display_options diff --git a/mathesar/rpc/utils.py b/mathesar/rpc/utils.py index 7bc33919d0..4ae3a0dc4e 100644 --- a/mathesar/rpc/utils.py +++ b/mathesar/rpc/utils.py @@ -2,12 +2,13 @@ from mathesar.models.base import Database -def connect(db_id): +def connect(db_id, user): """ Return a psycopg connection, given a Database model id. Args: db_id: The Django id corresponding to the Database. """ + print("User is: ", user) db_model = Database.current_objects.get(id=db_id) return get_psycopg_connection(db_model) From 45f3994d2982bbdbf8361ada4b6deefa3e2b48cd Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 6 May 2024 15:44:42 +0800 Subject: [PATCH 19/28] add classes to specify type options and defaults --- docs/docs/api/rpc.md | 2 + mathesar/rpc/columns.py | 103 ++++++++++++++++++++++++++++++++++------ 2 files changed, 91 insertions(+), 14 deletions(-) diff --git a/docs/docs/api/rpc.md b/docs/docs/api/rpc.md index 8f4230eea9..deffab54b5 100644 --- a/docs/docs/api/rpc.md +++ b/docs/docs/api/rpc.md @@ -53,6 +53,8 @@ To use an RPC function: members: - list - ColumnInfo + - TypeOptions + - ColumnDefault ## Responses diff --git a/mathesar/rpc/columns.py b/mathesar/rpc/columns.py index fa89cba65e..387a905d4e 100644 --- a/mathesar/rpc/columns.py +++ b/mathesar/rpc/columns.py @@ -12,28 +12,104 @@ from mathesar.utils.columns import get_display_options +class TypeOptions(TypedDict, total=False): + """ + Options applied to a type. All attributes are optional. + + Take special care with the difference between numeric and date/time + types w.r.t. precision. The attribute has a different meaning + depending on the type to which it's being applied. + + Attributes: + precision: For numeric types, the number of significant digits. + For date/time types, the number of fractional digits. + scale: For numeric types, the number of fractional digits. + fields: Which time fields are stored. See Postgres docs. + length: The maximum length of a character-type field. + item_type: The member type for arrays. + """ + precision: int + scale: int + fields: str + length: int + item_type: str + + @classmethod + def from_dict(cls, type_options): + if type_options is not None: + # All keys are optional, but we want to validate the keys + # we actually return. + all_keys = dict( + precision=type_options.get("precision"), + scale=type_options.get("scale"), + fields=type_options.get("fields"), + length=type_options.get("length"), + item_type=type_options.get("item_type"), + ) + return cls(**{k: v for k, v in all_keys.items() if v is not None}) + + +class ColumnDefault(TypedDict): + """ + A dictionary describing the default value for a column. + + Attributes: + value: An SQL expression giving the default value. + is_dynamic: Whether the `value` is possibly dynamic. + """ + value: str + is_dynamic: bool + + @classmethod + def from_dict(cls, col_default): + if col_default is not None: + return cls( + value=col_default["value"], + is_dynamic=col_default["is_dynamic"], + ) + + class ColumnInfo(TypedDict): """ Information about a column. Attributes: - id (int): The `attnum` of the column in the table. - name (str): The name of the column. - type (str): The type of the column on the database. - type_options (dict): The options applied to the column type. - nullable (bool): Whether or not the column is nullable. - primary_key (bool): Whether the column is in the primary key. - default (dict): The default value and whether it's dynamic. - has_dependents (bool): Whether the column has dependent objects. - description (str): The description of the column. + id: The `attnum` of the column in the table. + name: The name of the column. + type: The type of the column on the database. + type_options: The options applied to the column type. + nullable: Whether or not the column is nullable. + primary_key: Whether the column is in the primary key. + default: The default value and whether it's dynamic. + has_dependents: Whether the column has dependent objects. + description: The description of the column. """ + id: int + name: str + type: str + type_options: TypeOptions + nullable: bool + primary_key: bool + default: ColumnDefault + has_dependents: bool + description: str @classmethod - def from_column_info_json(cls, col_info): - return cls(**col_info) + def from_dict(cls, col_info): + return cls( + id=col_info["id"], + name=col_info["name"], + type=col_info["type"], + type_options=TypeOptions.from_dict(col_info.get("type_options")), + nullable=col_info["nullable"], + primary_key=col_info["primary_key"], + default=ColumnDefault.from_dict(col_info.get("default")), + has_dependents=col_info["has_dependents"], + description=col_info.get("description") + ) -@rpc_method(name='columns.list') +@rpc_method(name="columns.list") @http_basic_auth_login_required @handle_rpc_exceptions def list(*, table_oid: int, database_id: int, **kwargs): @@ -49,11 +125,10 @@ def list(*, table_oid: int, database_id: int, **kwargs): Returns: A list of column details, and a separate list of display options. """ - # TODO Add user as arg for connect and get_display_options request = kwargs.get(REQUEST_KEY) with connect(database_id, request.user) as conn: column_info = [ - ColumnInfo.from_column_info_json(col) + ColumnInfo.from_dict(col) for col in get_column_info_for_table(table_oid, conn) ] if request.user.metadata_privileges(database_id) is not None: From 41ff1b27729958c94b34d50a6b57604ff8d54058 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 6 May 2024 16:20:48 +0800 Subject: [PATCH 20/28] add class describing return value overall --- docs/docs/api/rpc.md | 3 ++- mathesar/rpc/columns.py | 26 +++++++++++++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/docs/docs/api/rpc.md b/docs/docs/api/rpc.md index deffab54b5..12097fb5b1 100644 --- a/docs/docs/api/rpc.md +++ b/docs/docs/api/rpc.md @@ -51,7 +51,8 @@ To use an RPC function: ::: mathesar.rpc.columns options: members: - - list + - list_ + - ColumnListReturn - ColumnInfo - TypeOptions - ColumnDefault diff --git a/mathesar/rpc/columns.py b/mathesar/rpc/columns.py index 387a905d4e..d5bbaf8bd5 100644 --- a/mathesar/rpc/columns.py +++ b/mathesar/rpc/columns.py @@ -109,18 +109,30 @@ def from_dict(cls, col_info): ) +class ColumnListReturn(TypedDict): + """ + Information about the columns of a table. + + Attributes: + column_info: Column information from the user's database. + display_options: Display metadata managed by Mathesar. + """ + column_info: list[ColumnInfo] + display_options: list[dict] + + @rpc_method(name="columns.list") @http_basic_auth_login_required @handle_rpc_exceptions -def list(*, table_oid: int, database_id: int, **kwargs): +def list_(*, table_oid: int, database_id: int, **kwargs): """ - List columns for a table, with information about each. + List information about columns for a table. Exposed as `list`. Also return display options for each column, if they're defined. Args: table_oid: Identity of the table in the user's database. - database_id: The Django model id of the database containing the table. + database_id: The Django id of the database containing the table. Returns: A list of column details, and a separate list of display options. @@ -136,7 +148,7 @@ def list(*, table_oid: int, database_id: int, **kwargs): display_options = get_display_options(table_oid, attnums) else: display_options = None - return { - "column_info": column_info, - "display_options": display_options - } + return ColumnListReturn( + column_info=column_info, + display_options=display_options, + ) From 4f585cdf8bd35fd1dd7fefb569e0423a391fc332 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 6 May 2024 23:35:36 +0800 Subject: [PATCH 21/28] add tests for wiring of RPC functions --- mathesar/rpc/columns.py | 2 + mathesar/rpc/exceptions/handlers.py | 1 + mathesar/tests/rpc/test_endpoints.py | 89 ++++++++++++++++++++-------- 3 files changed, 67 insertions(+), 25 deletions(-) diff --git a/mathesar/rpc/columns.py b/mathesar/rpc/columns.py index d5bbaf8bd5..159a1fc4e6 100644 --- a/mathesar/rpc/columns.py +++ b/mathesar/rpc/columns.py @@ -145,6 +145,8 @@ def list_(*, table_oid: int, database_id: int, **kwargs): ] if request.user.metadata_privileges(database_id) is not None: attnums = [col['id'] for col in column_info] + # TODO Use TypedDict spec pattern (above) for display_options once we + # have that model defined. display_options = get_display_options(table_oid, attnums) else: display_options = None diff --git a/mathesar/rpc/exceptions/handlers.py b/mathesar/rpc/exceptions/handlers.py index 1a45d38d25..a385bac318 100644 --- a/mathesar/rpc/exceptions/handlers.py +++ b/mathesar/rpc/exceptions/handlers.py @@ -8,6 +8,7 @@ def handle_rpc_exceptions(f): """Wrap a function to process any Exception raised.""" + f.rpc_exceptions_handled = True @wraps(f) def safe_func(*args, **kwargs): try: diff --git a/mathesar/tests/rpc/test_endpoints.py b/mathesar/tests/rpc/test_endpoints.py index 45cc870158..3767e80211 100644 --- a/mathesar/tests/rpc/test_endpoints.py +++ b/mathesar/tests/rpc/test_endpoints.py @@ -1,31 +1,70 @@ -from importlib import import_module - - -def test_rpc_endpoint_consistency_with_function_names( - live_server, - admin_client -): - """ - Tests whether the names of registered RPC endpoints align with the - corresponding Python methods. - - Fixtures: - live_server(pytest-django): Provides the url for django testing server. - admin_client(pytest-django): Provides a client logged in as a superuser. - """ - list_methods = admin_client.post( - path=f'{live_server.url}/api/rpc/v0/', +""" +This file tests wiring between the RPC endpoint and functions. + +Fixtures: + live_server(pytest-django): Provides the url for django testing server. + admin_client(pytest-django): Provides a client logged in as a superuser. +""" +import pytest +from modernrpc.core import Protocol +from modernrpc.auth import user_is_authenticated, user_is_superuser + +from mathesar.rpc import columns +from mathesar.rpc import connections + + +exposed_functions = [ + "columns.list", + "connections.add_from_known_connection", + "connections.add_from_scratch", +] + + +def test_rpc_endpoint_expected_methods(live_server, admin_client): + """Smoketest checks that we have exposed the expected methods.""" + all_methods = admin_client.post( + path=f"{live_server.url}/api/rpc/v0/", data={ "id": 1, "method": "system.listMethods", "jsonrpc": "2.0" }, content_type="application/json" - ) - methods = list_methods.json()['result'] - for method in methods: - if not method.startswith('system.'): - module_name, function_name = method.split('.') - mod = import_module(f"mathesar.rpc.{module_name}") - assert hasattr(mod, function_name) - assert callable(getattr(mod, function_name)) + ).json()["result"] + mathesar_methods = [m for m in all_methods if not m.startswith("system.")] + expect_methods = [ + "columns.list", + "connections.add_from_known_connection", + "connections.add_from_scratch", + ] + assert sorted(mathesar_methods) == expect_methods + + +@pytest.mark.parametrize( + "func,exposed_name,auth_pred_params", + [ + ( + columns.list_, + "columns.list", + [user_is_authenticated] + ), + ( + connections.add_from_known_connection, + "connections.add_from_known_connection", + [user_is_superuser] + ), + ( + connections.add_from_scratch, + "connections.add_from_scratch", + [user_is_superuser] + ) + ] +) +def test_correctly_exposed(func, exposed_name, auth_pred_params): + """Tests to make sure every RPC function is correctly wired up.""" + # Make sure we didn't typo the function names for the endpoint. + assert func.modernrpc_enabled is True + assert func.modernrpc_name == exposed_name + # Make sure other decorators are set as expected. + assert func.rpc_exceptions_handled is True + assert func.modernrpc_auth_predicates_params == [auth_pred_params] From 3a5a3946a1e32d3d462d326ff51b5de041b2847d Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 7 May 2024 13:57:50 +0800 Subject: [PATCH 22/28] move permission check to display_option function --- mathesar/rpc/columns.py | 53 ++++++++++++++++++++------------------- mathesar/utils/columns.py | 20 +++++++++------ 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/mathesar/rpc/columns.py b/mathesar/rpc/columns.py index 159a1fc4e6..d8a0f66dce 100644 --- a/mathesar/rpc/columns.py +++ b/mathesar/rpc/columns.py @@ -9,7 +9,7 @@ from db.columns.operations.select import get_column_info_for_table from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions from mathesar.rpc.utils import connect -from mathesar.utils.columns import get_display_options +from mathesar.utils.columns import get_raw_display_options class TypeOptions(TypedDict, total=False): @@ -36,17 +36,20 @@ class TypeOptions(TypedDict, total=False): @classmethod def from_dict(cls, type_options): - if type_options is not None: - # All keys are optional, but we want to validate the keys - # we actually return. - all_keys = dict( - precision=type_options.get("precision"), - scale=type_options.get("scale"), - fields=type_options.get("fields"), - length=type_options.get("length"), - item_type=type_options.get("item_type"), - ) - return cls(**{k: v for k, v in all_keys.items() if v is not None}) + if type_options is None: + return + # All keys are optional, but we want to validate the keys we + # actually return. + all_keys = dict( + precision=type_options.get("precision"), + scale=type_options.get("scale"), + fields=type_options.get("fields"), + length=type_options.get("length"), + item_type=type_options.get("item_type"), + ) + reduced_keys = {k: v for k, v in all_keys.items() if v is not None} + if reduced_keys != {}: + return cls(**reduced_keys) class ColumnDefault(TypedDict): @@ -124,7 +127,7 @@ class ColumnListReturn(TypedDict): @rpc_method(name="columns.list") @http_basic_auth_login_required @handle_rpc_exceptions -def list_(*, table_oid: int, database_id: int, **kwargs): +def list_(*, table_oid: int, database_id: int, **kwargs) -> ColumnListReturn: """ List information about columns for a table. Exposed as `list`. @@ -137,19 +140,17 @@ def list_(*, table_oid: int, database_id: int, **kwargs): Returns: A list of column details, and a separate list of display options. """ - request = kwargs.get(REQUEST_KEY) - with connect(database_id, request.user) as conn: - column_info = [ - ColumnInfo.from_dict(col) - for col in get_column_info_for_table(table_oid, conn) - ] - if request.user.metadata_privileges(database_id) is not None: - attnums = [col['id'] for col in column_info] - # TODO Use TypedDict spec pattern (above) for display_options once we - # have that model defined. - display_options = get_display_options(table_oid, attnums) - else: - display_options = None + user = kwargs.get(REQUEST_KEY).user + with connect(database_id, user) as conn: + raw_column_info = get_column_info_for_table(table_oid, conn) + column_info, attnums = tuple( + zip( + *[(ColumnInfo.from_dict(col), col['id']) for col in raw_column_info] + ) + ) + display_options = get_raw_display_options( + database_id, table_oid, attnums, user + ) return ColumnListReturn( column_info=column_info, display_options=display_options, diff --git a/mathesar/utils/columns.py b/mathesar/utils/columns.py index d6fb4b0cad..eb742ade68 100644 --- a/mathesar/utils/columns.py +++ b/mathesar/utils/columns.py @@ -2,11 +2,15 @@ # This should be replaced once we have the ColumnMetadata model sorted out. -def get_display_options(table_oid, attnums): - return [ - {"id": c.attnum} | c.display_options - for c in Column.current_objects.filter( - table__oid=table_oid, attnum__in=attnums - ) - if c.display_options is not None - ] +def get_raw_display_options(database_id, table_oid, attnums, user): + """Get display options for the columns from Django.""" + if user.metadata_privileges(database_id) is not None: + return [ + {"id": c.attnum} | c.display_options + for c in Column.current_objects.filter( + table__schema__database__id=database_id, + table__oid=table_oid, + attnum__in=attnums + ) + if c.display_options is not None + ] From b7c91d136316a08c342c97b85c19aa52e1ec6581 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 7 May 2024 14:00:36 +0800 Subject: [PATCH 23/28] add test for column lister RPC function --- mathesar/tests/rpc/test_columns.py | 155 +++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 mathesar/tests/rpc/test_columns.py diff --git a/mathesar/tests/rpc/test_columns.py b/mathesar/tests/rpc/test_columns.py new file mode 100644 index 0000000000..113b84a751 --- /dev/null +++ b/mathesar/tests/rpc/test_columns.py @@ -0,0 +1,155 @@ +from contextlib import contextmanager + +from mathesar.rpc import columns +from mathesar.models.users import User + + +def test_columns_list(rf, monkeypatch): + request = rf.post('/api/rpc/v0/', data={}) + request.user = User(username='alice', password='pass1234') + table_oid = 23457 + database_id = 2 + + @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_info(_table_oid, conn): + if _table_oid != table_oid: + raise AssertionError('incorrect parameters passed') + return [ + { + 'id': 1, 'name': 'id', 'type': 'integer', + 'default': {'value': 'identity', 'is_dynamic': True}, + 'nullable': False, 'description': None, 'primary_key': True, + 'type_options': None, + 'has_dependents': True + }, { + 'id': 2, 'name': 'numcol', 'type': 'numeric', + 'default': {'value': "'8'::numeric", 'is_dynamic': False}, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'scale': None, 'precision': None}, + 'has_dependents': False + }, { + 'id': 4, 'name': 'numcolmod', 'type': 'numeric', + 'default': None, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'scale': 3, 'precision': 5}, + 'has_dependents': False + }, { + 'id': 6, 'name': 'numarr', 'type': '_array', + 'default': None, + 'nullable': True, + 'description': 'a numeric array column', + 'primary_key': False, + 'type_options': {'scale': 2, 'item_type': 'numeric', 'precision': 8}, + 'has_dependents': False + }, { + 'id': 8, 'name': 'ivlcolmod', 'type': 'interval', + 'default': None, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'fields': 'day to second', 'precision': 3}, + 'has_dependents': False + }, { + 'id': 10, 'name': 'arrcol', 'type': '_array', + 'default': None, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'item_type': 'text'}, + 'has_dependents': False + }, { + 'id': 12, 'name': 'vccol', 'type': 'character varying', + 'default': None, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'length': 123}, + 'has_dependents': False + }, + ] + + def mock_display_options(_database_id, _table_oid, attnums, user): + if ( + database_id != 2 + or table_oid != 23457 + or attnums != (1, 2, 4, 6, 8, 10, 12) + or user.username != 'alice' + ): + raise AssertionError("incorrect parameters passed") + return [ + { + 'id': 4, + 'use_grouping': 'true', + 'number_format': 'english', + 'show_as_percentage': False, + 'maximum_fraction_digits': 2, + 'minimum_fraction_digits': 2 + } + ] + monkeypatch.setattr(columns, 'connect', mock_connect) + monkeypatch.setattr(columns, 'get_column_info_for_table', mock_column_info) + monkeypatch.setattr(columns, 'get_raw_display_options', mock_display_options) + expect_col_list = { + 'column_info': ( + { + 'id': 1, 'name': 'id', 'type': 'integer', + 'default': {'value': 'identity', 'is_dynamic': True}, + 'nullable': False, 'description': None, 'primary_key': True, + 'type_options': None, + 'has_dependents': True + }, { + 'id': 2, 'name': 'numcol', 'type': 'numeric', + 'default': {'value': "'8'::numeric", 'is_dynamic': False}, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': None, + 'has_dependents': False + }, { + 'id': 4, 'name': 'numcolmod', 'type': 'numeric', + 'default': None, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'scale': 3, 'precision': 5}, + 'has_dependents': False + }, { + 'id': 6, 'name': 'numarr', 'type': '_array', + 'default': None, + 'nullable': True, + 'description': 'a numeric array column', + 'primary_key': False, + 'type_options': {'scale': 2, 'item_type': 'numeric', 'precision': 8}, + 'has_dependents': False + }, { + 'id': 8, 'name': 'ivlcolmod', 'type': 'interval', + 'default': None, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'fields': 'day to second', 'precision': 3}, + 'has_dependents': False + }, { + 'id': 10, 'name': 'arrcol', 'type': '_array', + 'default': None, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'item_type': 'text'}, + 'has_dependents': False + }, { + 'id': 12, 'name': 'vccol', 'type': 'character varying', + 'default': None, + 'nullable': True, 'description': None, 'primary_key': False, + 'type_options': {'length': 123}, + 'has_dependents': False + } + ), + 'display_options': [ + { + 'id': 4, + 'use_grouping': 'true', + 'number_format': 'english', + 'show_as_percentage': False, + 'maximum_fraction_digits': 2, + 'minimum_fraction_digits': 2 + } + ] + } + actual_col_list = columns.list_(table_oid=23457, database_id=2, request=request) + assert actual_col_list == expect_col_list From 68b5ce01318d9992a15b2a26fb681f342677a07c Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 7 May 2024 14:00:58 +0800 Subject: [PATCH 24/28] fix linting issues --- mathesar/rpc/exceptions/handlers.py | 1 + mathesar/tests/rpc/test_endpoints.py | 1 - mathesar/utils/columns.py | 6 +++--- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mathesar/rpc/exceptions/handlers.py b/mathesar/rpc/exceptions/handlers.py index a385bac318..62203230f8 100644 --- a/mathesar/rpc/exceptions/handlers.py +++ b/mathesar/rpc/exceptions/handlers.py @@ -9,6 +9,7 @@ def handle_rpc_exceptions(f): """Wrap a function to process any Exception raised.""" f.rpc_exceptions_handled = True + @wraps(f) def safe_func(*args, **kwargs): try: diff --git a/mathesar/tests/rpc/test_endpoints.py b/mathesar/tests/rpc/test_endpoints.py index 3767e80211..41841079b1 100644 --- a/mathesar/tests/rpc/test_endpoints.py +++ b/mathesar/tests/rpc/test_endpoints.py @@ -6,7 +6,6 @@ admin_client(pytest-django): Provides a client logged in as a superuser. """ import pytest -from modernrpc.core import Protocol from modernrpc.auth import user_is_authenticated, user_is_superuser from mathesar.rpc import columns diff --git a/mathesar/utils/columns.py b/mathesar/utils/columns.py index eb742ade68..b172d63203 100644 --- a/mathesar/utils/columns.py +++ b/mathesar/utils/columns.py @@ -8,9 +8,9 @@ def get_raw_display_options(database_id, table_oid, attnums, user): return [ {"id": c.attnum} | c.display_options for c in Column.current_objects.filter( - table__schema__database__id=database_id, - table__oid=table_oid, - attnum__in=attnums + table__schema__database__id=database_id, + table__oid=table_oid, + attnum__in=attnums ) if c.display_options is not None ] From 9278b6f2f220216340194069b609b8f086a2b5e8 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 7 May 2024 15:34:54 +0800 Subject: [PATCH 25/28] reduce test scope, improve focus --- mathesar/tests/rpc/test_columns.py | 46 +++++++----------------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/mathesar/tests/rpc/test_columns.py b/mathesar/tests/rpc/test_columns.py index 113b84a751..e2ecce34f3 100644 --- a/mathesar/tests/rpc/test_columns.py +++ b/mathesar/tests/rpc/test_columns.py @@ -33,7 +33,9 @@ def mock_column_info(_table_oid, conn): }, { 'id': 2, 'name': 'numcol', 'type': 'numeric', 'default': {'value': "'8'::numeric", 'is_dynamic': False}, - 'nullable': True, 'description': None, 'primary_key': False, + 'nullable': True, + 'description': 'My super numeric column', + 'primary_key': False, 'type_options': {'scale': None, 'precision': None}, 'has_dependents': False }, { @@ -42,31 +44,17 @@ def mock_column_info(_table_oid, conn): 'nullable': True, 'description': None, 'primary_key': False, 'type_options': {'scale': 3, 'precision': 5}, 'has_dependents': False - }, { - 'id': 6, 'name': 'numarr', 'type': '_array', - 'default': None, - 'nullable': True, - 'description': 'a numeric array column', - 'primary_key': False, - 'type_options': {'scale': 2, 'item_type': 'numeric', 'precision': 8}, - 'has_dependents': False }, { 'id': 8, 'name': 'ivlcolmod', 'type': 'interval', 'default': None, 'nullable': True, 'description': None, 'primary_key': False, - 'type_options': {'fields': 'day to second', 'precision': 3}, + 'type_options': {'fields': 'day to second'}, 'has_dependents': False }, { 'id': 10, 'name': 'arrcol', 'type': '_array', 'default': None, 'nullable': True, 'description': None, 'primary_key': False, - 'type_options': {'item_type': 'text'}, - 'has_dependents': False - }, { - 'id': 12, 'name': 'vccol', 'type': 'character varying', - 'default': None, - 'nullable': True, 'description': None, 'primary_key': False, - 'type_options': {'length': 123}, + 'type_options': {'item_type': 'character varying', 'length': 3}, 'has_dependents': False }, ] @@ -75,7 +63,7 @@ def mock_display_options(_database_id, _table_oid, attnums, user): if ( database_id != 2 or table_oid != 23457 - or attnums != (1, 2, 4, 6, 8, 10, 12) + or attnums != (1, 2, 4, 8, 10) or user.username != 'alice' ): raise AssertionError("incorrect parameters passed") @@ -103,7 +91,9 @@ def mock_display_options(_database_id, _table_oid, attnums, user): }, { 'id': 2, 'name': 'numcol', 'type': 'numeric', 'default': {'value': "'8'::numeric", 'is_dynamic': False}, - 'nullable': True, 'description': None, 'primary_key': False, + 'nullable': True, + 'description': 'My super numeric column', + 'primary_key': False, 'type_options': None, 'has_dependents': False }, { @@ -112,31 +102,17 @@ def mock_display_options(_database_id, _table_oid, attnums, user): 'nullable': True, 'description': None, 'primary_key': False, 'type_options': {'scale': 3, 'precision': 5}, 'has_dependents': False - }, { - 'id': 6, 'name': 'numarr', 'type': '_array', - 'default': None, - 'nullable': True, - 'description': 'a numeric array column', - 'primary_key': False, - 'type_options': {'scale': 2, 'item_type': 'numeric', 'precision': 8}, - 'has_dependents': False }, { 'id': 8, 'name': 'ivlcolmod', 'type': 'interval', 'default': None, 'nullable': True, 'description': None, 'primary_key': False, - 'type_options': {'fields': 'day to second', 'precision': 3}, + 'type_options': {'fields': 'day to second'}, 'has_dependents': False }, { 'id': 10, 'name': 'arrcol', 'type': '_array', 'default': None, 'nullable': True, 'description': None, 'primary_key': False, - 'type_options': {'item_type': 'text'}, - 'has_dependents': False - }, { - 'id': 12, 'name': 'vccol', 'type': 'character varying', - 'default': None, - 'nullable': True, 'description': None, 'primary_key': False, - 'type_options': {'length': 123}, + 'type_options': {'item_type': 'character varying', 'length': 3}, 'has_dependents': False } ), From 847ded497e54fb55ffa7be390a97e15312866e5e Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 7 May 2024 15:56:40 +0800 Subject: [PATCH 26/28] add test for wiring of column info getter to DB --- db/tests/columns/operations/test_select.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/db/tests/columns/operations/test_select.py b/db/tests/columns/operations/test_select.py index 242a8e6847..44c995766f 100644 --- a/db/tests/columns/operations/test_select.py +++ b/db/tests/columns/operations/test_select.py @@ -1,9 +1,11 @@ +from unittest.mock import patch import warnings import pytest from sqlalchemy import ( String, Integer, Column, Table, MetaData, DateTime, func ) from db.columns.exceptions import DynamicDefaultWarning +from db.columns.operations import select as col_select from db.columns.operations.select import ( get_column_attnum_from_name, get_column_default, get_column_name_from_attnum, get_columns_attnum_from_names, @@ -13,6 +15,14 @@ from db.metadata import get_empty_metadata +def test_get_column_info_for_table(): + with patch.object(col_select, 'exec_msar_func') as mock_exec: + mock_exec.return_value.fetchone = lambda: ('a', 'b') + result = col_select.get_column_info_for_table('table', 'conn') + mock_exec.assert_called_once_with('conn', 'get_column_info', 'table') + assert result == 'a' + + def test_get_attnum_from_name(engine_with_schema): engine, schema = engine_with_schema table_name = "table_with_columns" From 616965337774a6f986bb91d3fcfa702fdc46ce91 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Thu, 9 May 2024 14:26:52 +0800 Subject: [PATCH 27/28] document fixtures in test_columns.py --- mathesar/tests/rpc/test_columns.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/mathesar/tests/rpc/test_columns.py b/mathesar/tests/rpc/test_columns.py index e2ecce34f3..14aff7c760 100644 --- a/mathesar/tests/rpc/test_columns.py +++ b/mathesar/tests/rpc/test_columns.py @@ -1,3 +1,10 @@ +""" +This file tests the column listing function. + +Fixtures: + rf(pytest-django): Provides mocked `Request` objects. + admin_client(pytest): Lets you monkeypatch an object for testing. +""" from contextlib import contextmanager from mathesar.rpc import columns From 7740f76f141ed3f3092c988e27503d4cd3418cd3 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Thu, 9 May 2024 17:26:28 +0800 Subject: [PATCH 28/28] fix fixture name typo --- mathesar/tests/rpc/test_columns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mathesar/tests/rpc/test_columns.py b/mathesar/tests/rpc/test_columns.py index 14aff7c760..9b77cb3e93 100644 --- a/mathesar/tests/rpc/test_columns.py +++ b/mathesar/tests/rpc/test_columns.py @@ -3,7 +3,7 @@ Fixtures: rf(pytest-django): Provides mocked `Request` objects. - admin_client(pytest): Lets you monkeypatch an object for testing. + monkeypatch(pytest): Lets you monkeypatch an object for testing. """ from contextlib import contextmanager