Skip to content

Commit

Permalink
fix: fix output of RPCs returning scalar values with multiple rows
Browse files Browse the repository at this point in the history
resolves #1584

BREAKING CHANGE:
Changed output of RPCs to match return type better:
* single scalar return: value (unchanged)
* setof scalar return: array of values (new; was array of objects)
* single composite return: object (new; was array of objects)
* setof composite return: array of objects (unchanged)
* void return: nothing (new; was "null")

A single OUT column is now treated as "composite" instead of "scalar",
i.e. consistent with multiple OUT columns.
  • Loading branch information
wolfgangwalther committed Dec 10, 2020
1 parent 093fd3c commit 0c25f12
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 99 deletions.
14 changes: 10 additions & 4 deletions src/PostgREST/App.hs
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,8 @@ app dbStructure conf apiRequest =
preferParams = iPreferParameters apiRequest
pq = requestToCallProcQuery (QualifiedIdentifier pdSchema pdName) (specifiedProcArgs (iColumns apiRequest) proc)
(iPayload apiRequest) returnsScalar preferParams returning
stm = callProcStatement returnsScalar pq q cq shouldCount (contentType == CTSingularJSON)
(contentType == CTTextCSV) (contentType `elem` rawContentTypes) (preferParams == Just MultipleObjects)
bField pgVer prepared
stm = callProcStatement returnsScalar returnsSingle pq q cq shouldCount (contentType == CTSingularJSON)
(contentType == CTTextCSV) (preferParams == Just MultipleObjects) bField pgVer prepared
row <- H.statement mempty stm
let (tableTotal, queryTotal, body, gucHeaders, gucStatus) = row
gucs = (,) <$> gucHeaders <*> gucStatus
Expand Down Expand Up @@ -351,6 +350,10 @@ app dbStructure conf apiRequest =
case iTarget apiRequest of
TargetProc proc _ -> procReturnsScalar proc
_ -> False
returnsSingle =
case iTarget apiRequest of
TargetProc proc _ -> procReturnsSingle proc
_ -> False
pgVer = pgVersion dbStructure
profileH = contentProfileH <$> iProfile apiRequest

Expand Down Expand Up @@ -401,7 +404,10 @@ responseContentTypeOrError accepts rawContentTypes action target = serves conten
-}
binaryField :: ContentType -> [ContentType] -> Bool -> ReadRequest -> Either Response (Maybe FieldName)
binaryField ct rawContentTypes isScalarProc readReq
| isScalarProc = Right Nothing
| isScalarProc =
if ct `elem` rawContentTypes
then Right $ Just "pgrst_scalar"
else Right Nothing
| ct `elem` rawContentTypes =
let fieldName = headMay fldNames in
if length fldNames == 1 && fieldName /= Just "*"
Expand Down
8 changes: 6 additions & 2 deletions src/PostgREST/DbStructure.hs
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,12 @@ procsSqlQuery = [q|
tn.nspname AS schema,
COALESCE(comp.relname, t.typname) AS name,
p.proretset AS rettype_is_setof,
-- Only pg pseudo type that is a row type is 'record'
(t.typtype = 'c' or t.typtype = 'p' and t.typname = 'record') AS rettype_is_composite,
(t.typtype = 'c'
-- Only pg pseudo type that is a row type is 'record'
or t.typtype = 'p' and t.typname = 'record'
-- if any INOUT or OUT arguments present, treat as composite
or COALESCE(proargmodes::text[] && '{b,o}', false)
) AS rettype_is_composite,
p.provolatile
FROM pg_proc p
JOIN pg_namespace pn ON pn.oid = p.pronamespace
Expand Down
14 changes: 9 additions & 5 deletions src/PostgREST/Private/QueryFragment.hs
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,15 @@ asCsvF = asCsvHeaderF <> " || '\n' || " <> asCsvBodyF
")"
asCsvBodyF = "coalesce(string_agg(substring(_postgrest_t::text, 2, length(_postgrest_t::text) - 2), '\n'), '')"

asJsonF :: SqlFragment
asJsonF = "coalesce(json_agg(_postgrest_t), '[]')::character varying"

asJsonSingleF :: SqlFragment --TODO! unsafe when the query actually returns multiple rows, used only on inserting and returning single element
asJsonSingleF = "coalesce(string_agg(row_to_json(_postgrest_t)::text, ','), '')::character varying "
asJsonF :: Bool -> SqlFragment
asJsonF returnsScalar
| returnsScalar = "coalesce(json_agg(_postgrest_t.pgrst_scalar), '[]')::character varying"
| otherwise = "coalesce(json_agg(_postgrest_t), '[]')::character varying"

asJsonSingleF :: Bool -> SqlFragment --TODO! unsafe when the query actually returns multiple rows, used only on inserting and returning single element
asJsonSingleF returnsScalar
| returnsScalar = "coalesce(string_agg(to_json(_postgrest_t.pgrst_scalar)::text, ','), '')::character varying"
| otherwise = "coalesce(string_agg(to_json(_postgrest_t)::text, ','), '')::character varying"

asBinaryF :: FieldName -> SqlFragment
asBinaryF fieldName = "coalesce(string_agg(_postgrest_t." <> pgFmtIdent fieldName <> ", ''), '')"
Expand Down
28 changes: 12 additions & 16 deletions src/PostgREST/Statements.hs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ createWriteStatement selectQuery mutateQuery wantSingle isInsert asCsv rep pKeys
bodyF
| rep `elem` [None, HeadersOnly] = "''"
| asCsv = asCsvF
| wantSingle = asJsonSingleF
| otherwise = asJsonF
| wantSingle = asJsonSingleF False
| otherwise = asJsonF False

selectF
-- prevent using any of the column names in ?select= when no response is returned from the CTE
Expand Down Expand Up @@ -108,9 +108,9 @@ createReadStatement selectQuery countQuery isSingle countTotal asCsv binaryField

bodyF
| asCsv = asCsvF
| isSingle = asJsonSingleF
| isSingle = asJsonSingleF False
| isJust binaryField = asBinaryF $ fromJust binaryField
| otherwise = asJsonF
| otherwise = asJsonF False

decodeStandard :: HD.Result ResultsWithCount
decodeStandard =
Expand All @@ -128,10 +128,10 @@ standardRow = (,,,,,) <$> nullableColumn HD.int8 <*> column HD.int8

type ProcResults = (Maybe Int64, Int64, ByteString, Either SimpleError [GucHeader], Either SimpleError (Maybe Status))

callProcStatement :: Bool -> H.Snippet -> H.Snippet -> H.Snippet -> Bool ->
Bool -> Bool -> Bool -> Bool -> Maybe FieldName -> PgVersion -> Bool ->
callProcStatement :: Bool -> Bool -> H.Snippet -> H.Snippet -> H.Snippet -> Bool ->
Bool -> Bool -> Bool -> Maybe FieldName -> PgVersion -> Bool ->
H.Statement () ProcResults
callProcStatement returnsScalar callProcQuery selectQuery countQuery countTotal isSingle asCsv asBinary multObjects binaryField pgVer =
callProcStatement returnsScalar returnsSingle callProcQuery selectQuery countQuery countTotal asSingle asCsv multObjects binaryField pgVer =
H.dynamicallyParameterized snippet decodeProc
where
snippet =
Expand All @@ -149,16 +149,12 @@ callProcStatement returnsScalar callProcQuery selectQuery countQuery countTotal
(countCTEF, countResultF) = countF countQuery countTotal

bodyF
| returnsScalar = scalarBodyF
| isSingle = asJsonSingleF
| asCsv = asCsvF
| asSingle = asJsonSingleF returnsScalar
| asCsv = asCsvF
| isJust binaryField = asBinaryF $ fromJust binaryField
| otherwise = asJsonF

scalarBodyF
| asBinary = asBinaryF "pgrst_scalar"
| multObjects = "json_agg(_postgrest_t.pgrst_scalar)::character varying"
| otherwise = "(json_agg(_postgrest_t.pgrst_scalar)->0)::character varying"
| returnsSingle
&& not multObjects = asJsonSingleF returnsScalar
| otherwise = asJsonF returnsScalar

decodeProc :: HD.Result ProcResults
decodeProc =
Expand Down
6 changes: 6 additions & 0 deletions src/PostgREST/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,14 @@ specifiedProcArgs keys proc =
procReturnsScalar :: ProcDescription -> Bool
procReturnsScalar proc = case proc of
ProcDescription{pdReturnType = (Single (Scalar _))} -> True
ProcDescription{pdReturnType = (SetOf (Scalar _))} -> True
_ -> False

procReturnsSingle :: ProcDescription -> Bool
procReturnsSingle proc = case proc of
ProcDescription{pdReturnType = (Single _)} -> True
_ -> False

procTableName :: ProcDescription -> Maybe TableName
procTableName proc = case pdReturnType proc of
SetOf (Composite qi) -> Just $ qiName qi
Expand Down
24 changes: 16 additions & 8 deletions test/Feature/QuerySpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -827,14 +827,22 @@ spec actualPgVersion = do
it "fails if a single column is not selected" $ do
request methodGet "/images?select=img,name&name=eq.A.png" (acceptHdrs "application/octet-stream") ""
`shouldRespondWith`
[json| {"message":"application/octet-stream requested but more than one column was selected"} |]
{ matchStatus = 406
, matchHeaders = [matchContentTypeJson]
}
request methodGet "/images?select=*&name=eq.A.png" (acceptHdrs "application/octet-stream") ""
`shouldRespondWith` 406
request methodGet "/images?name=eq.A.png" (acceptHdrs "application/octet-stream") ""
`shouldRespondWith` 406
[json| {"message":"application/octet-stream requested but more than one column was selected"} |]
{ matchStatus = 406 }

request methodGet "/images?select=*&name=eq.A.png"
(acceptHdrs "application/octet-stream")
""
`shouldRespondWith`
[json| {"message":"application/octet-stream requested but more than one column was selected"} |]
{ matchStatus = 406 }

request methodGet "/images?name=eq.A.png"
(acceptHdrs "application/octet-stream")
""
`shouldRespondWith`
[json| {"message":"application/octet-stream requested but more than one column was selected"} |]
{ matchStatus = 406 }

it "concatenates results if more than one row is returned" $
request methodGet "/images_base64?select=img&name=in.(A.png,B.png)" (acceptHdrs "application/octet-stream") ""
Expand Down
Loading

0 comments on commit 0c25f12

Please sign in to comment.