remove(feat): fail schema cache lookup with invalid db-schemas config

Closes #4364 by reverting 86c3257

As discussed on #4364, 86c3257
generates several problems:

1)We diverge from database as source of truth because once a
  schema is dropped, it's not automatically removed from `db-schemas`
  and then postgREST fails.
2)We diverge from PostgreSQL `search_path` lenient behavior:
```sql
set search_path to anything, here, even, nonexistent; -- this doesn't fail
```
3)schema-based multitenancy becomes more complicated because of 1.

There are no noticeable gains from 86c3257:

- No gains in performance for the schema cache queries.
- No user report about it ever being a problem. In fact the leniency was
  a feature.

Author: steve-chavez

src/PostgREST/Query/SqlFragment.hs

@@ -595,8 +595,14 @@ handlerF rout = \case
   NoAgg                      -> "''::text"
 
 schemaDescription :: Text -> SQL.Snippet
-schemaDescription schema =
-  "SELECT pg_catalog.obj_description(" <> encoded <> "::regnamespace, 'pg_namespace')"
+schemaDescription schema = SQL.sql (encodeUtf8 [trimming|
+  SELECT
+    description
+  FROM
+    pg_namespace n
+    left join pg_description d on d.objoid = n.oid
+  WHERE
+    n.nspname = |]) <> encoded
   where
     encoded = SQL.encoderAndParam (HE.nonNullable HE.unknown) $ encodeUtf8 schema
 
@@ -608,7 +614,7 @@ accessibleTables schema = SQL.sql (encodeUtf8 [trimming|
   FROM pg_class c
   JOIN pg_namespace n ON n.oid = c.relnamespace
   WHERE c.relkind IN ('v','r','m','f','p')
-  AND c.relnamespace = |]) <> encodedSchema <> "::regnamespace " <> SQL.sql (encodeUtf8 [trimming|
+  AND n.nspname = |]) <> encodedSchema <> " " <> SQL.sql (encodeUtf8 [trimming|
   AND (
     pg_has_role(c.relowner, 'USAGE')
     or has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER')
@@ -620,7 +626,7 @@ accessibleTables schema = SQL.sql (encodeUtf8 [trimming|
     encodedSchema = SQL.encoderAndParam (HE.nonNullable HE.text) schema
 
 accessibleFuncs :: Text -> SQL.Snippet
-accessibleFuncs schema = baseFuncSqlQuery <> "AND p.pronamespace = " <> encodedSchema <> "::regnamespace"
+accessibleFuncs schema = baseFuncSqlQuery <> "AND pn.nspname = " <> encodedSchema
   where
     encodedSchema = SQL.encoderAndParam (HE.nonNullable HE.text) schema
 

src/PostgREST/SchemaCache.hs

@@ -42,7 +42,6 @@ import NeatInterpolation          (trimming)
 import PostgREST.Config                      (AppConfig (..))
 import PostgREST.Config.Database             (TimezoneNames,
                                               toIsolationLevel)
-import PostgREST.Query.SqlFragment           (escapeIdent)
 import PostgREST.SchemaCache.Identifiers     (FieldName,
                                               QualifiedIdentifier (..),
                                               RelIdentifier (..),
@@ -357,7 +356,7 @@ allFunctions :: Bool -> SQL.Statement AppConfig RoutineMap
 allFunctions = SQL.Statement funcsSqlQuery params decodeFuncs
   where
     params =
-      (map escapeIdent . toList . configDbSchemas >$< arrayParam HE.text) <>
+      (toList . configDbSchemas >$< arrayParam HE.text) <>
       (configDbHoistedTxSettings >$< arrayParam HE.text)
 
 baseTypesCte :: Text
@@ -458,7 +457,7 @@ funcsSqlQuery = encodeUtf8 [trimming|
   ) func_settings ON TRUE
   WHERE t.oid <> 'trigger'::regtype AND COALESCE(a.callable, true)
   AND prokind = 'f'
-  AND p.pronamespace = ANY($$1::regnamespace[]) |]
+  AND pn.nspname = ANY($$1) |]
 {-
 Adds M2O and O2O relationships for views to tables, tables to views, and views to views. The example below is taken from the test fixtures, but the views names/colnames were modified.
 
@@ -569,7 +568,7 @@ addViewPrimaryKeys tabs keyDeps =
 allTables :: Bool -> SQL.Statement AppConfig TablesMap
 allTables = SQL.Statement tablesSqlQuery params decodeTables
   where
-    params = map escapeIdent . toList . configDbSchemas >$< arrayParam HE.text
+    params = toList . configDbSchemas >$< arrayParam HE.text
 
 -- | Gets tables with their PK cols
 tablesSqlQuery :: SqlQuery
@@ -621,6 +620,8 @@ tablesSqlQuery =
               ON a.attrelid = ad.adrelid AND a.attnum = ad.adnum
           JOIN pg_class c
               ON a.attrelid = c.oid
+          JOIN pg_namespace nc
+              ON c.relnamespace = nc.oid
           JOIN pg_type t
               ON a.atttypid = t.oid
           LEFT JOIN base_types bt
@@ -632,7 +633,7 @@ tablesSqlQuery =
           AND a.attnum > 0
           AND NOT a.attisdropped
           AND c.relkind in ('r', 'v', 'f', 'm', 'p')
-          AND c.relnamespace = ANY($$1::regnamespace[])
+          AND nc.nspname = ANY($$1)
   ),
   columns_agg AS (
     SELECT
@@ -812,8 +813,8 @@ allViewsKeyDependencies =
   --  * json transformation: https://gist.github.com/wolfgangwalther/3a8939da680c24ad767e93ad2c183089
   where
     params =
-      (map escapeIdent . toList . configDbSchemas >$< arrayParam HE.text) <>
-      (map escapeIdent . toList . configDbExtraSearchPath >$< arrayParam HE.text)
+      (toList . configDbSchemas >$< arrayParam HE.text) <>
+      (configDbExtraSearchPath >$< arrayParam HE.text)
     sql = encodeUtf8 [trimming|
       with recursive
       pks_fks as (
@@ -844,18 +845,17 @@ allViewsKeyDependencies =
       views as (
         select
           c.oid          as view_id,
-          c.relnamespace as view_schema_id,
           n.nspname      as view_schema,
           c.relname      as view_name,
           r.ev_action    as view_definition
         from pg_class c
         join pg_namespace n on n.oid = c.relnamespace
         join pg_rewrite r on r.ev_class = c.oid
-        where c.relkind in ('v', 'm') and c.relnamespace = ANY($$1::regnamespace[] || $$2::regnamespace[])
+        where c.relkind in ('v', 'm') and n.nspname = ANY($$1 || $$2)
       ),
       transform_json as (
         select
-          view_id, view_schema_id, view_schema, view_name,
+          view_id, view_schema, view_name,
           -- the following formatting is without indentation on purpose
           -- to allow simple diffs, with less whitespace noise
           replace(
@@ -935,31 +935,30 @@ allViewsKeyDependencies =
       ),
       target_entries as(
         select
-          view_id, view_schema_id, view_schema, view_name,
+          view_id, view_schema, view_name,
           json_array_elements(view_definition->0->'targetList') as entry
         from transform_json
       ),
       results as(
         select
-          view_id, view_schema_id, view_schema, view_name,
+          view_id, view_schema, view_name,
           (entry->>'resno')::int as view_column,
           (entry->>'resorigtbl')::oid as resorigtbl,
           (entry->>'resorigcol')::int as resorigcol
         from target_entries
       ),
       -- CYCLE detection according to PG docs: https://www.postgresql.org/docs/current/queries-with.html#QUERIES-WITH-CYCLE
       -- Can be replaced with CYCLE clause once PG v13 is EOL.
-      recursion(view_id, view_schema_id, view_schema, view_name, view_column, resorigtbl, resorigcol, is_cycle, path) as(
+      recursion(view_id, view_schema, view_name, view_column, resorigtbl, resorigcol, is_cycle, path) as(
         select
           r.*,
           false,
           ARRAY[resorigtbl]
         from results r
-        where view_schema_id = ANY ($$1::regnamespace[])
+        where view_schema = ANY ($$1)
         union all
         select
           view.view_id,
-          view.view_schema_id,
           view.view_schema,
           view.view_name,
           view.view_column,
@@ -1018,7 +1017,7 @@ mediaHandlers :: Bool -> SQL.Statement AppConfig MediaHandlerMap
 mediaHandlers =
   SQL.Statement sql params decodeMediaHandlers
   where
-    params = map escapeIdent . toList . configDbSchemas >$< arrayParam HE.text
+    params = toList . configDbSchemas >$< arrayParam HE.text
     sql = encodeUtf8 [trimming|
       with
       all_relations as (
@@ -1059,7 +1058,7 @@ mediaHandlers =
         join pg_type      arg_name     on arg_name.oid = proc.proargtypes[0]
         join pg_namespace arg_schema   on arg_schema.oid = arg_name.typnamespace
       where
-        proc.pronamespace = ANY($$1::regnamespace[]) and
+        proc_schema.nspname = ANY($$1) and
         proc.pronargs = 1 and
         arg_name.oid in (select reltype from all_relations)
       union
@@ -1075,7 +1074,7 @@ mediaHandlers =
         join media_types mtype on proc.prorettype = mtype.oid
         join pg_namespace typ_sch     on typ_sch.oid = mtype.typnamespace
       where
-        proc.pronamespace = ANY($$1::regnamespace[]) and NOT proretset
+        pro_sch.nspname = ANY($$1) and NOT proretset
         and prokind = 'f'|]
 
 decodeMediaHandlers :: HD.Result MediaHandlerMap

test/io/test_io.py

@@ -1051,8 +1051,10 @@ def drain_stdout(proc):
         )
         infinite_recursion_5xx_regx = r'.+: WITH pgrst_source AS.+SELECT "public"\."infinite_recursion"\.\* FROM "public"\."infinite_recursion".+_postgrest_t'
         root_tables_regx = r".+: SELECT   n.nspname AS table_schema, .+ FROM pg_class c .+ ORDER BY table_schema, table_name"
-        root_procs_regx = r".+: WITH base_types AS \(.+\) SELECT   pn.nspname AS proc_schema, .+ FROM pg_proc p.+AND p.pronamespace = \$1::regnamespace"
-        root_descr_regx = r".+: SELECT pg_catalog\.obj_description\(\$1::regnamespace, 'pg_namespace'\)"
+        root_procs_regx = r".+: WITH base_types AS \(.+\) SELECT   pn.nspname AS proc_schema, .+ FROM pg_proc p.+AND pn.nspname = \$1"
+        root_descr_regx = (
+            r".+: SELECT.+description.+FROM.+pg_namespace n.+WHERE.+n.nspname =\$1"
+        )
         set_config_regx = (
             r".+: select set_config\('search_path', \$1, true\), set_config\("
         )
@@ -2010,23 +2012,30 @@ def test_allow_configs_to_be_set_to_empty(defaultenv):
         assert response.status_code == 200
 
 
-def test_schema_cache_error_observation(defaultenv):
+def test_schema_cache_error_observation(defaultenv, metapostgrest):
     "schema cache error observation should be logged with invalid db-schemas or db-extra-search-path"
 
+    role = "timeout_authenticator"
+
     env = {
         **defaultenv,
-        "PGRST_DB_EXTRA_SEARCH_PATH": "x",
+        "PGUSER": role,
+        "PGRST_DB_ANON_ROLE": role,
+        "PGRST_INTERNAL_SCHEMA_CACHE_SLEEP": "500",
     }
 
     with run(env=env, no_startup_stdout=False, wait_for_readiness=False) as postgrest:
         # TODO: postgrest should exit here, instead it keeps retrying
         # exitCode = wait_until_exit(postgrest)
         # assert exitCode == 1
+        set_statement_timeout(metapostgrest, role, 400)
+
+        output = postgrest.read_stdout(nlines=10)
 
-        output = postgrest.read_stdout(nlines=9)
         assert (
-            "Failed to load the schema cache using db-schemas=public and db-extra-search-path=x"
-            in output[7]
+            "Failed to load the schema cache using db-schemas=public and db-extra-search-path=public"
+            in line
+            for line in output
         )
 
 

test/spec/Feature/Query/ErrorSpec.hs

@@ -10,6 +10,59 @@ import Test.Hspec.Wai.JSON
 import Protolude  hiding (get)
 import SpecHelper
 
+nonExistentSchema :: SpecWith ((), Application)
+nonExistentSchema = do
+  describe "Non existent api schema" $ do
+    it "succeeds when requesting root path" $
+      get "/" `shouldRespondWith` 200
+
+    it "gives 404 when requesting a nonexistent table in this nonexistent schema" $
+      get "/nonexistent_table" `shouldRespondWith` 404
+
+  describe "Non existent URL" $ do
+    it "gives 404 on a single nested route" $
+      get "/projects/nested" `shouldRespondWith` 404
+
+    it "gives 404 on a double nested route" $
+      get "/projects/nested/double" `shouldRespondWith` 404
+
+  describe "Unsupported HTTP methods" $ do
+    it "should return 405 for CONNECT method" $
+      request methodConnect "/"
+          []
+          ""
+        `shouldRespondWith`
+          [json|
+            {"hint": null,
+             "details": null,
+             "code": "PGRST117",
+             "message":"Unsupported HTTP method: CONNECT"}|]
+          { matchStatus = 405 }
+
+    it "should return 405 for TRACE method" $
+      request methodTrace "/"
+          []
+          ""
+        `shouldRespondWith`
+          [json|
+            {"hint": null,
+             "details": null,
+             "code": "PGRST117",
+             "message":"Unsupported HTTP method: TRACE"}|]
+          { matchStatus = 405 }
+
+    it "should return 405 for OTHER method" $
+      request "OTHER" "/"
+          []
+          ""
+        `shouldRespondWith`
+          [json|
+            {"hint": null,
+             "details": null,
+             "code": "PGRST117",
+             "message":"Unsupported HTTP method: OTHER"}|]
+          { matchStatus = 405 }
+
 pgErrorCodeMapping :: SpecWith ((), Application)
 pgErrorCodeMapping = do
   describe "PostreSQL error code mappings" $ do

test/spec/Main.hs

@@ -126,6 +126,7 @@ main = do
 
       extraSearchPathApp   = appDbs testCfgExtraSearchPath
       unicodeApp           = appDbs testUnicodeCfg
+      nonexistentSchemaApp = appDbs testNonexistentSchemaCfg
       multipleSchemaApp    = appDbs testMultipleSchemaCfg
       ignorePrivOpenApi    = appDbs testIgnorePrivOpenApiCfg
 
@@ -221,6 +222,10 @@ main = do
     parallel $ before asymJwkSetApp $
       describe "Feature.Auth.AsymmetricJwtSpec" Feature.Auth.AsymmetricJwtSpec.spec
 
+    -- this test runs with a nonexistent db-schema
+    parallel $ before nonexistentSchemaApp $
+      describe "Feature.Query.NonExistentSchemaErrorSpec" Feature.Query.ErrorSpec.nonExistentSchema
+
     -- this test runs with an extra search path
     parallel $ before extraSearchPathApp $ do
       describe "Feature.ExtraSearchPathSpec" Feature.ExtraSearchPathSpec.spec

test/spec/SpecHelper.hs

@@ -238,6 +238,9 @@ testCfgAsymJWKSet =
   , configJWKS = rightToMaybe $ parseSecret secret
   }
 
+testNonexistentSchemaCfg :: AppConfig
+testNonexistentSchemaCfg = baseCfg { configDbSchemas = fromList ["nonexistent"] }
+
 testCfgExtraSearchPath :: AppConfig
 testCfgExtraSearchPath = baseCfg { configDbExtraSearchPath = ["public", "extensions", "EXTRA \"@/\\#~_-"] }