fix: handle queries on non-existing table gracefully

This commit is contained in:
Taimoor Zaeem
2025-02-21 13:49:54 -05:00
committed by GitHub
parent 9c880c082a
commit 390ba19932
13 changed files with 125 additions and 58 deletions
+2
View File
@@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
+ Fixed `"column <json_aggregate>.<alias> does not exist"` error when selecting `?select=...table(aias:count())`
- #3727, Clarify "listening" logs - @steve-chavez
- #3795, Clarify `Accept: vnd.pgrst.object` error message - @steve-chavez
- #3697, #3602, Handle queries on non-existing table gracefully - @taimoorzaeem
### Changed
@@ -40,6 +41,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
+ This preference was deprecated in favor of Functions with an array of JSON objects
- #3013, Drop support for Limited updates/deletes
+ The feature was complicated and largely unused.
- #3697, #3602, Querying non-existent table now returns `PGRST205` error instead of empty json - @taimoorzaeem
## [12.2.8] - 2025-02-10
+4
View File
@@ -286,6 +286,10 @@ Related to a :ref:`schema_cache`. Most of the time, these errors are solved by :
| | | in the ``columns`` query parameter is not found. |
| PGRST204 | | |
+---------------+-------------+-------------------------------------------------------------+
| .. _pgrst205: | 404 | Caused when the :ref:`table specified <tables_views>` in |
| | | the URI is not found. |
| PGRST205 | | |
+---------------+-------------+-------------------------------------------------------------+
.. _pgrst3**:
+21
View File
@@ -45,6 +45,7 @@ import PostgREST.SchemaCache.Relationship (Cardinality (..),
RelationshipsMap)
import PostgREST.SchemaCache.Routine (Routine (..),
RoutineParam (..))
import PostgREST.SchemaCache.Table (Table (..))
import Protolude
@@ -82,6 +83,7 @@ data ApiRequestError
| UnacceptableSchema [Text]
| UnsupportedMethod ByteString
| ColumnNotFound Text Text
| TableNotFound Text Text [Table]
| GucHeadersError
| GucStatusError
| PutMatchingPkError
@@ -128,6 +130,7 @@ instance PgrstError ApiRequestError where
status UnacceptableSchema{} = HTTP.status406
status UnsupportedMethod{} = HTTP.status405
status ColumnNotFound{} = HTTP.status400
status TableNotFound{} = HTTP.status404
status GucHeadersError = HTTP.status500
status GucStatusError = HTTP.status500
status PutMatchingPkError = HTTP.status400
@@ -285,6 +288,12 @@ instance JSON.ToJSON ApiRequestError where
toJSON (ColumnNotFound relName colName) = toJsonPgrstError
SchemaCacheErrorCode04 ("Could not find the '" <> colName <> "' column of '" <> relName <> "' in the schema cache") Nothing Nothing
toJSON (TableNotFound schemaName relName tbls) = toJsonPgrstError
SchemaCacheErrorCode05
("Could not find the table '" <> schemaName <> "." <> relName <> "' in the schema cache")
Nothing
(JSON.String <$> tableNotFoundHint schemaName relName tbls)
-- |
-- If no relationship is found then:
--
@@ -382,6 +391,16 @@ noRpcHint schema procName params allProcs overloadedProcs =
| null overloadedProcs = Fuzzy.getOne fuzzySetOfProcs procName
| otherwise = (procName <>) <$> Fuzzy.getOne fuzzySetOfParams (listToText params)
-- |
-- Do a fuzzy search in all tables in the same schema and return closest result
tableNotFoundHint :: Text -> Text -> [Table] -> Maybe Text
tableNotFoundHint schema tblName tblList
= fmap (\tbl -> "Perhaps you meant the table '" <> schema <> "." <> tbl <> "'") perhapsTable
where
perhapsTable = Fuzzy.getOne fuzzyTableSet tblName
fuzzyTableSet = Fuzzy.fromList [ tableName tbl | tbl <- tblList, tableSchema tbl == schema]
compressedRel :: Relationship -> JSON.Value
-- An ambiguousness error cannot happen for computed relationships TODO refactor so this mempty is not needed
compressedRel ComputedRelationship{} = JSON.object mempty
@@ -672,6 +691,7 @@ data ErrorCode
| SchemaCacheErrorCode02
| SchemaCacheErrorCode03
| SchemaCacheErrorCode04
| SchemaCacheErrorCode05
-- JWT authentication errors
| JWTErrorCode00
| JWTErrorCode01
@@ -719,6 +739,7 @@ buildErrorCode code = case code of
SchemaCacheErrorCode02 -> "PGRST202"
SchemaCacheErrorCode03 -> "PGRST203"
SchemaCacheErrorCode04 -> "PGRST204"
SchemaCacheErrorCode05 -> "PGRST205"
JWTErrorCode00 -> "PGRST300"
JWTErrorCode01 -> "PGRST301"
+17 -8
View File
@@ -153,18 +153,20 @@ dbActionPlan dbAct conf apiReq sCache = case dbAct of
wrappedReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Bool -> Either Error CrudPlan
wrappedReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{..},..} headersOnly = do
rPlan <- readPlan identifier conf sCache apiRequest
(handler, mediaType) <- mapLeft ApiRequestError $ negotiateContent conf apiRequest identifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
qi <- mapLeft ApiRequestError $ findTable identifier (dbTables sCache)
rPlan <- readPlan qi conf sCache apiRequest
(handler, mediaType) <- mapLeft ApiRequestError $ negotiateContent conf apiRequest qi iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right ()
return $ WrappedReadPlan rPlan SQL.Read handler mediaType headersOnly identifier
return $ WrappedReadPlan rPlan SQL.Read handler mediaType headersOnly qi
mutateReadPlan :: Mutation -> ApiRequest -> QualifiedIdentifier -> AppConfig -> SchemaCache -> Either Error CrudPlan
mutateReadPlan mutation apiRequest@ApiRequest{iPreferences=Preferences{..},..} identifier conf sCache = do
rPlan <- readPlan identifier conf sCache apiRequest
mPlan <- mutatePlan mutation identifier apiRequest sCache rPlan
qi <- mapLeft ApiRequestError $ findTable identifier (dbTables sCache)
rPlan <- readPlan qi conf sCache apiRequest
mPlan <- mutatePlan mutation qi apiRequest sCache rPlan
if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right ()
(handler, mediaType) <- mapLeft ApiRequestError $ negotiateContent conf apiRequest identifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
return $ MutateReadPlan rPlan mPlan SQL.Write handler mediaType mutation identifier
(handler, mediaType) <- mapLeft ApiRequestError $ negotiateContent conf apiRequest qi iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
return $ MutateReadPlan rPlan mPlan SQL.Write handler mediaType mutation qi
callReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> InvokeMethod -> Either Error CallReadPlan
callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{preferHandling, invalidPrefs},..} invMethod = do
@@ -745,6 +747,13 @@ validateAggFunctions aggFunctionsAllowed (Node rp@ReadPlan {select} forest)
| not aggFunctionsAllowed && any (isJust . csAggFunction) select = Left AggregatesNotAllowed
| otherwise = Node rp <$> traverse (validateAggFunctions aggFunctionsAllowed) forest
-- | Lookup table in the schema cache before creating read plan
findTable :: QualifiedIdentifier -> TablesMap -> Either ApiRequestError QualifiedIdentifier
findTable qi@QualifiedIdentifier{..} tableMap =
case HM.lookup qi tableMap of
Nothing -> Left (TableNotFound qiSchema qiName (HM.elems tableMap))
Just _ -> Right qi
addFilters :: ResolverContext -> ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
addFilters ctx ApiRequest{..} rReq =
foldr addFilterToNode (Right rReq) flts
@@ -967,7 +976,7 @@ mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{
typedColumnsOrError = resolveOrError ctx tbl `traverse` S.toList iColumns
resolveOrError :: ResolverContext -> Maybe Table -> FieldName -> Either ApiRequestError CoercibleField
resolveOrError _ Nothing _ = Left NotFound
resolveOrError _ Nothing _ = Left NotFound -- TODO: control never reaches here since #3869, should be fixed when fixing #3906
resolveOrError ctx (Just table) field =
case resolveTableFieldName table field Nothing of
CoercibleField{cfIRType=""} -> Left $ ColumnNotFound (tableName table) field
+1
View File
@@ -25,6 +25,7 @@ instance Hashable RelIdentifier
-- | Represents a pg identifier with a prepended schema name "schema.table".
-- When qiSchema is "", the schema is defined by the pg search_path.
-- TODO: Refactor this, we also use QI for procedure names
data QualifiedIdentifier = QualifiedIdentifier
{ qiSchema :: Schema
, qiName :: TableName
+31 -4
View File
@@ -7,8 +7,8 @@ from util import *
from postgrest import *
def test_requests_wait_for_schema_cache_reload(defaultenv):
"requests that use the schema cache (e.g. resource embedding) wait for the schema cache to reload"
def test_requests_with_resource_embedding_wait_for_schema_cache_reload(defaultenv):
"requests that use the schema cache with resource embedding wait long for the schema cache to reload"
env = {
**defaultenv,
@@ -34,6 +34,33 @@ def test_requests_wait_for_schema_cache_reload(defaultenv):
assert plan_dur > 10000.0
def test_requests_without_resource_embedding_wait_for_schema_cache_reload(defaultenv):
"requests that use the schema cache without resource embedding wait less for the schema cache to reload"
env = {
**defaultenv,
"PGRST_DB_SCHEMAS": "apflora",
"PGRST_DB_POOL": "2",
"PGRST_DB_ANON_ROLE": "postgrest_test_anonymous",
"PGRST_SERVER_TIMING_ENABLED": "true",
}
with run(env=env, wait_max_seconds=30) as postgrest:
# reload the schema cache
response = postgrest.session.get("/rpc/notify_pgrst")
assert response.status_code == 204
postgrest.wait_until_scache_starts_loading()
response = postgrest.session.get("/tpopmassn")
assert response.status_code == 200
plan_dur = parse_server_timings_header(response.headers["Server-Timing"])[
"plan"
]
assert plan_dur < 10000.0
# TODO: This test fails now because of https://github.com/PostgREST/postgrest/pull/2122
# The stack size of 1K(-with-rtsopts=-K1K) is not enough and this fails with "stack overflow"
# A stack size of 200K seems to be enough for succeess
@@ -65,6 +92,6 @@ def test_should_not_fail_with_stack_overflow(defaultenv):
with run(env=env, wait_max_seconds=30) as postgrest:
response = postgrest.session.get("/unknown-table?select=unknown-rel(*)")
assert response.status_code == 400
assert response.status_code == 404
data = response.json()
assert data["code"] == "PGRST200"
assert data["code"] == "PGRST205"
+12 -20
View File
@@ -973,11 +973,10 @@ def test_log_level(level, defaultenv):
r'- - postgrest_test_anonymous \[.+\] "GET /unknown HTTP/1.1" 404 - "" "python-requests/.+"',
output[2],
)
assert len(output) == 5
assert "Connection" and "is available" in output[3]
assert "Connection" and "is available" in output[4]
assert "Connection" and "is used" in output[5]
assert "Connection" and "is used" in output[6]
assert len(output) == 7
assert "Connection" and "is used" in output[4]
@pytest.mark.parametrize("level", ["crit", "error", "warn", "info", "debug"])
@@ -999,15 +998,11 @@ def test_log_query(level, defaultenv):
response = postgrest.session.get("/projects")
assert response.status_code == 200
response = postgrest.session.get("/unknown")
assert response.status_code == 404
response = postgrest.session.get("/infinite_recursion")
assert response.status_code == 500
root_2xx_regx = r'.+: WITH pgrst_source AS.+SELECT "public"\."root"\(\) pgrst_scalar.+_postgrest_t'
get_2xx_regx = r'.+: WITH pgrst_source AS.+SELECT "public"\."projects"\.\* FROM "public"\."projects".+_postgrest_t'
unknown_4xx_regx = r'.+: WITH pgrst_source AS.+SELECT "public"\."unknown"\.\* FROM "public"\."unknown".+_postgrest_t'
infinite_recursion_5xx_regx = r'.+: WITH pgrst_source AS.+SELECT "public"\."infinite_recursion"\.\* FROM "public"\."infinite_recursion".+_postgrest_t'
if level == "crit":
@@ -1018,26 +1013,23 @@ def test_log_query(level, defaultenv):
assert re.match(infinite_recursion_5xx_regx, output[1])
assert len(output) == 3
elif level == "warn":
output = postgrest.read_stdout(nlines=6)
assert re.match(unknown_4xx_regx, output[0])
assert re.match(infinite_recursion_5xx_regx, output[3])
assert len(output) == 5
output = postgrest.read_stdout(nlines=2)
assert re.match(infinite_recursion_5xx_regx, output[1])
assert len(output) == 2
elif level == "info":
output = postgrest.read_stdout(nlines=10)
output = postgrest.read_stdout(nlines=6)
assert re.match(root_2xx_regx, output[0])
assert re.match(get_2xx_regx, output[2])
assert re.match(unknown_4xx_regx, output[4])
assert re.match(infinite_recursion_5xx_regx, output[7])
assert len(output) == 9
assert re.match(infinite_recursion_5xx_regx, output[5])
assert len(output) == 6
elif level == "debug":
output_ok = postgrest.read_stdout(nlines=8)
assert re.match(root_2xx_regx, output_ok[2])
assert re.match(get_2xx_regx, output_ok[6])
assert len(output_ok) == 8
output_err = postgrest.read_stdout(nlines=10)
assert re.match(unknown_4xx_regx, output_err[2])
assert re.match(infinite_recursion_5xx_regx, output_err[7])
assert len(output_err) == 9
output_err = postgrest.read_stdout(nlines=4)
assert re.match(infinite_recursion_5xx_regx, output_err[3])
assert len(output_err) == 4
def test_no_pool_connection_required_on_bad_http_logic(defaultenv):
+2 -6
View File
@@ -24,12 +24,8 @@ spec =
it "should not raise 'transaction in progress' error" $
raceTest 10 $
get "/fakefake"
`shouldRespondWith` [json|
{ "hint": null,
"details":null,
"code":"42P01",
"message":"relation \"test.fakefake\" does not exist"
} |]
`shouldRespondWith`
[json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.factories'","message":"Could not find the table 'test.fakefake' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = []
}
+6 -1
View File
@@ -109,7 +109,12 @@ spec =
context "totally unknown route" $
it "fails with 404" $
request methodDelete "/foozle?id=eq.101" [] "" `shouldRespondWith` 404
request methodDelete "/foozle?id=eq.101" [] ""
`shouldRespondWith`
[json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.foo'","message":"Could not find the table 'test.foozle' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = []
}
context "table with limited privileges" $ do
it "fails deleting the row when return=representation and selecting all the columns" $
+1 -1
View File
@@ -477,7 +477,7 @@ spec actualPgVersion = do
{"id": 204, "body": "yyy"},
{"id": 205, "body": "zzz"}]|]
`shouldRespondWith`
[json|{} |]
[json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.articles'","message":"Could not find the table 'test.garlic' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = []
}
@@ -64,7 +64,13 @@ spec =
}
it "doesn't find another_table in schema v1" $
request methodGet "/another_table" [("Accept-Profile", "v1")] "" `shouldRespondWith` 404
request methodGet "/another_table"
[("Accept-Profile", "v1")] ""
`shouldRespondWith`
[json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'v1.another_table' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = []
}
it "fails trying to read table from unkown schema" $
request methodGet "/parents" [("Accept-Profile", "unkown")] "" `shouldRespondWith`
+14 -15
View File
@@ -24,7 +24,12 @@ spec = do
describe "Querying a nonexistent table" $
it "causes a 404" $
get "/faketable" `shouldRespondWith` 404
get "/faketable"
`shouldRespondWith`
[json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.private_table'","message":"Could not find the table 'test.faketable' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = []
}
describe "Filtering response" $ do
it "matches with equality" $
@@ -819,14 +824,12 @@ spec = do
, matchHeaders = [matchContentTypeJson]
}
it "cannot request a partitioned table as parent from a partition" $
-- we only search for foreign key relationships after checking the
-- the existence of first table, #3869
it "table not found error if first table does not exist" $
get "/car_model_sales_202101?select=id,name,car_models(id,name)&order=id.asc" `shouldRespondWith`
[json|
{"hint":"Perhaps you meant 'car_model_sales' instead of 'car_model_sales_202101'.",
"details":"Searched for a foreign key relationship between 'car_model_sales_202101' and 'car_models' in the schema 'test', but no matches were found.",
"code":"PGRST200",
"message":"Could not find a relationship between 'car_model_sales_202101' and 'car_models' in the schema cache"} |]
{ matchStatus = 400
[json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.car_model_sales'","message":"Could not find the table 'test.car_model_sales_202101' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = [matchContentTypeJson]
}
@@ -841,14 +844,10 @@ spec = do
, matchHeaders = [matchContentTypeJson]
}
it "cannot request partitioned tables as children from a partition" $
it "table not found error if first table does not exist" $
get "/car_models_default?select=id,name,car_model_sales(id,name)&order=id.asc" `shouldRespondWith`
[json|
{"hint":"Perhaps you meant 'car_model_sales' instead of 'car_models_default'.",
"details":"Searched for a foreign key relationship between 'car_models_default' and 'car_model_sales' in the schema 'test', but no matches were found.",
"code":"PGRST200",
"message":"Could not find a relationship between 'car_models_default' and 'car_model_sales' in the schema cache"} |]
{ matchStatus = 400
[json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.car_model_sales'","message":"Could not find the table 'test.car_models_default' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = [matchContentTypeJson]
}
+7 -2
View File
@@ -17,7 +17,12 @@ spec = do
it "indicates no table found by returning 404" $
request methodPatch "/fake" []
[json| { "real": false } |]
`shouldRespondWith` 404
`shouldRespondWith`
[json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.factories'","message":"Could not find the table 'test.fake' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = []
}
context "on an empty table" $
it "succeeds with status code 204" $
@@ -343,7 +348,7 @@ spec = do
{"id": 204, "body": "yyy"},
{"id": 205, "body": "zzz"}]|]
`shouldRespondWith`
[json|{} |]
[json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.articles'","message":"Could not find the table 'test.garlic' in the schema cache"} |]
{ matchStatus = 404
, matchHeaders = []
}