From 902e4e8921779346adc19ff6690d03820be2b797 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Thu, 21 Apr 2022 17:31:03 -0500 Subject: [PATCH] refactor: clean allColumns query * Add OpenAPI test to ensure enum labels are present * Remove unnecessary fk/pk columns cte * Remove unnecessary step for parsing enum labels --- src/PostgREST/DbStructure.hs | 159 +++++++++-------------- test/spec/Feature/OpenApi/OpenApiSpec.hs | 18 +++ 2 files changed, 76 insertions(+), 101 deletions(-) diff --git a/src/PostgREST/DbStructure.hs b/src/PostgREST/DbStructure.hs index 657ab60b9..09f80f492 100644 --- a/src/PostgREST/DbStructure.hs +++ b/src/PostgREST/DbStructure.hs @@ -36,7 +36,6 @@ import qualified Hasql.Statement as SQL import qualified Hasql.Transaction as SQL import Contravariant.Extras (contrazip2) -import Data.Text (split) import Text.InterpolatedString.Perl6 (q) import PostgREST.Config.Database (pgVersionStatement) @@ -154,7 +153,7 @@ decodeColumns tables = <*> column HD.text <*> nullableColumn HD.int4 <*> nullableColumn HD.text - <*> nullableColumn HD.text + <*> arrayColumn HD.text decodeRels :: HD.Result [Relationship] decodeRels = @@ -543,7 +542,57 @@ allColumns :: TablesMap -> Bool -> SQL.Statement [Schema] [Column] allColumns tabs = SQL.Statement sql (arrayParam HE.text) (decodeColumns tabs) where + -- CTE based on information_schema.columns changed: remove the owner filter sql = [q| + WITH + columns AS ( + SELECT + nc.nspname::name AS table_schema, + c.relname::name AS table_name, + a.attname::name AS column_name, + d.description AS description, + pg_get_expr(ad.adbin, ad.adrelid)::text AS column_default, + not (a.attnotnull OR t.typtype = 'd' AND t.typnotnull) AS is_nullable, + CASE + WHEN t.typtype = 'd' THEN + CASE + WHEN bt.typelem <> 0::oid AND bt.typlen = (-1) THEN 'ARRAY'::text + WHEN nbt.nspname = 'pg_catalog'::name THEN format_type(t.typbasetype, NULL::integer) + ELSE format_type(a.atttypid, a.atttypmod) + END + ELSE + CASE + WHEN t.typelem <> 0::oid AND t.typlen = (-1) THEN 'ARRAY'::text + WHEN nt.nspname = 'pg_catalog'::name THEN format_type(a.atttypid, NULL::integer) + ELSE format_type(a.atttypid, a.atttypmod) + END + END::text AS data_type, + information_schema._pg_char_max_length( + information_schema._pg_truetypid(a.*, t.*), + information_schema._pg_truetypmod(a.*, t.*) + )::integer AS character_maximum_length, + COALESCE(bt.typname, t.typname)::name AS udt_name, + a.attnum::integer AS position + FROM pg_attribute a + LEFT JOIN pg_description AS d + ON d.objoid = a.attrelid and d.objsubid = a.attnum + LEFT JOIN pg_attrdef ad + ON a.attrelid = ad.adrelid AND a.attnum = ad.adnum + JOIN (pg_class c JOIN pg_namespace nc ON c.relnamespace = nc.oid) + ON a.attrelid = c.oid + JOIN (pg_type t JOIN pg_namespace nt ON t.typnamespace = nt.oid) + ON a.atttypid = t.oid + LEFT JOIN (pg_type bt JOIN pg_namespace nbt ON bt.typnamespace = nbt.oid) + ON t.typtype = 'd' AND t.typbasetype = bt.oid + LEFT JOIN (pg_collation co JOIN pg_namespace nco ON co.collnamespace = nco.oid) + ON a.attcollation = co.oid AND (nco.nspname <> 'pg_catalog'::name OR co.collname <> 'default'::name) + WHERE + NOT pg_is_other_temp_schema(nc.oid) + AND a.attnum > 0 + AND NOT a.attisdropped + AND c.relkind in ('r', 'v', 'f', 'm', 'p') + AND nc.nspname = ANY($1) + ) SELECT DISTINCT info.table_schema AS schema, info.table_name AS table_name, @@ -553,101 +602,9 @@ allColumns tabs = info.data_type AS col_type, info.character_maximum_length AS max_len, info.column_default AS default_value, - array_to_string(enum_info.vals, ',') AS enum, + coalesce(enum_info.vals, '{}') AS enum, info.position - FROM ( - -- CTE based on pg_catalog to get PRIMARY/FOREIGN key and UNIQUE columns outside api schema - WITH key_columns AS ( - SELECT - r.oid AS r_oid, - c.oid AS c_oid, - n.nspname, - c.relname, - r.conname, - r.contype, - unnest(r.conkey) AS conkey - FROM - pg_constraint r, - pg_class c, - pg_namespace n - WHERE - r.contype IN ('f', 'p', 'u') - AND c.relkind IN ('r', 'v', 'f', 'm', 'p') - AND r.conrelid = c.oid - AND c.relnamespace = n.oid - AND n.nspname <> ANY (ARRAY['pg_catalog', 'information_schema'] || $1) - ), - /* - -- CTE based on information_schema.columns - -- changed: - -- remove the owner filter - -- limit columns to the ones in the api schema or PK/FK columns - */ - columns AS ( - SELECT - nc.nspname::name AS table_schema, - c.relname::name AS table_name, - a.attname::name AS column_name, - d.description AS description, - pg_get_expr(ad.adbin, ad.adrelid)::text AS column_default, - not (a.attnotnull OR t.typtype = 'd' AND t.typnotnull) AS is_nullable, - CASE - WHEN t.typtype = 'd' THEN - CASE - WHEN bt.typelem <> 0::oid AND bt.typlen = (-1) THEN 'ARRAY'::text - WHEN nbt.nspname = 'pg_catalog'::name THEN format_type(t.typbasetype, NULL::integer) - ELSE format_type(a.atttypid, a.atttypmod) - END - ELSE - CASE - WHEN t.typelem <> 0::oid AND t.typlen = (-1) THEN 'ARRAY'::text - WHEN nt.nspname = 'pg_catalog'::name THEN format_type(a.atttypid, NULL::integer) - ELSE format_type(a.atttypid, a.atttypmod) - END - END::text AS data_type, - information_schema._pg_char_max_length( - information_schema._pg_truetypid(a.*, t.*), - information_schema._pg_truetypmod(a.*, t.*) - )::integer AS character_maximum_length, - COALESCE(bt.typname, t.typname)::name AS udt_name, - a.attnum::integer AS position - FROM pg_attribute a - LEFT JOIN key_columns kc - ON kc.conkey = a.attnum AND kc.c_oid = a.attrelid - LEFT JOIN pg_description AS d - ON d.objoid = a.attrelid and d.objsubid = a.attnum - LEFT JOIN pg_attrdef ad - ON a.attrelid = ad.adrelid AND a.attnum = ad.adnum - JOIN (pg_class c JOIN pg_namespace nc ON c.relnamespace = nc.oid) - ON a.attrelid = c.oid - JOIN (pg_type t JOIN pg_namespace nt ON t.typnamespace = nt.oid) - ON a.atttypid = t.oid - LEFT JOIN (pg_type bt JOIN pg_namespace nbt ON bt.typnamespace = nbt.oid) - ON t.typtype = 'd' AND t.typbasetype = bt.oid - LEFT JOIN (pg_collation co JOIN pg_namespace nco ON co.collnamespace = nco.oid) - ON a.attcollation = co.oid AND (nco.nspname <> 'pg_catalog'::name OR co.collname <> 'default'::name) - WHERE - NOT pg_is_other_temp_schema(nc.oid) - AND a.attnum > 0 - AND NOT a.attisdropped - AND c.relkind in ('r', 'v', 'f', 'm', 'p') - -- Filter only columns that are FK/PK or in the api schema: - AND (nc.nspname = ANY ($1) OR kc.r_oid IS NOT NULL) - ) - SELECT - table_schema, - table_name, - column_name, - description, - is_nullable, - data_type, - character_maximum_length, - column_default, - udt_name, - position - FROM columns - WHERE table_schema NOT IN ('pg_catalog', 'information_schema') - ) AS info + FROM columns info LEFT OUTER JOIN ( SELECT n.nspname AS s, @@ -658,19 +615,19 @@ allColumns tabs = JOIN pg_namespace n ON n.oid = t.typnamespace GROUP BY s,n ) AS enum_info ON (info.udt_name = enum_info.n) - ORDER BY schema, position |] + WHERE info.table_schema NOT IN ('pg_catalog', 'information_schema') + ORDER BY schema, position; + |] columnFromRow :: TablesMap -> (Text, Text, Text, Maybe Text, Bool, Text, - Maybe Int32, Maybe Text, Maybe Text) + Maybe Int32, Maybe Text, [Text]) -> Maybe Column columnFromRow tabs (s, t, n, desc, nul, typ, l, d, e) = buildColumn <$> table where - buildColumn tbl = Column tbl n desc nul typ l d (parseEnum e) + buildColumn tbl = Column tbl n desc nul typ l d e table = M.lookup (QualifiedIdentifier s t) tabs - parseEnum :: Maybe Text -> [Text] - parseEnum = maybe [] (split (==',')) allM2ORels :: PgVersion -> Bool -> SQL.Statement () [Relationship] allM2ORels pgVer = diff --git a/test/spec/Feature/OpenApi/OpenApiSpec.hs b/test/spec/Feature/OpenApi/OpenApiSpec.hs index fe35310a4..1fb06bcff 100644 --- a/test/spec/Feature/OpenApi/OpenApiSpec.hs +++ b/test/spec/Feature/OpenApi/OpenApiSpec.hs @@ -514,6 +514,24 @@ spec actualPgVersion = describe "OpenAPI" $ do types `shouldBe` Just "13:00:00" + it "enum" $ do + r <- simpleBody <$> get "/" + + let types = r ^? key "definitions" . key "menagerie" . key "properties" . key "enum" + + liftIO $ + + types `shouldBe` Just [aesonQQ| + { + "format": "test.enum_menagerie_type", + "type": "string", + "enum": [ + "foo", + "bar" + ] + } + |] + describe "RPC" $ do it "includes function summary/description and body schema for arguments" $ do