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
2022-04-22 11:46:45 -05:00
parent d2719420f4
commit 902e4e8921
2 changed files with 76 additions and 101 deletions
@@ -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 =
@@ -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