Fix output of RPCs returning scalar values with multiple rows and/or …

…stale schema cache

CHANGELOG.md

@@ -26,6 +26,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #1636, Fix `application/octet-stream` appending `charset=utf-8` - @steve-chavez
  - #1615, Fix RPC return type handling and embedding for domains with composite base type - @wolfgangwalther
  - #1469, Fix overloading of functions with unnamed arguments - @wolfgangwalther
+ - #1584, Fix output of RPCs returning a set of scalar values - @wolfgangwalther
+ - #1615, Fix output of RPCs returning scalars with stale schema cache - @wolfgangwalther
 
 ### Changed
 

src/PostgREST/App.hs

@@ -287,8 +287,8 @@ app dbStructure conf apiRequest =
             Right (q, cq, bField, returning) -> do
               let
                 preferParams = iPreferParameters apiRequest
-                pq = requestToCallProcQuery (QualifiedIdentifier pdSchema pdName) (specifiedProcArgs (iColumns apiRequest) proc) returnsScalar preferParams returning
-                stm = callProcStatement returnsScalar pq q cq shouldCount (contentType == CTSingularJSON)
+                pq = requestToCallProcQuery (QualifiedIdentifier pdSchema pdName) (specifiedProcArgs (iColumns apiRequest) proc) preferParams returning
+                stm = callProcStatement pq q cq shouldCount (contentType == CTSingularJSON || returnsSingle)
                         (contentType == CTTextCSV) (contentType `elem` rawContentTypes) (preferParams == Just MultipleObjects)
                         bField pgVer
               row <- H.statement (toS $ pjRaw pJson) stm
@@ -337,9 +337,9 @@ app dbStructure conf apiRequest =
         plannedCount = iPreferCount apiRequest == Just PlannedCount
         shouldCount = exactCount || estimatedCount
         topLevelRange = iTopLevelRange apiRequest
-        returnsScalar =
+        returnsSingle =
           case iTarget apiRequest of
-            TargetProc proc _ -> procReturnsScalar proc
+            TargetProc proc _ -> procReturnsSingle proc
             _                 -> False
         pgVer = pgVersion dbStructure
         profileH = contentProfileH <$> iProfile apiRequest
@@ -353,7 +353,7 @@ app dbStructure conf apiRequest =
           (,,,) <$>
           (readRequestToQuery <$> readReq) <*>
           (readRequestToCountQuery <$> readReq) <*>
-          (binaryField contentType rawContentTypes returnsScalar =<< readReq) <*>
+          (binaryField contentType rawContentTypes (isJust $ iSelect apiRequest) (iTarget apiRequest) =<< readReq) <*>
           (returnings =<< readReq)
 
         mutateSqlParts s t =
@@ -389,9 +389,9 @@ responseContentTypeOrError accepts rawContentTypes action target = serves conten
   | If raw(binary) output is requested, check that ContentType is one of the admitted rawContentTypes and that
   | `?select=...` contains only one field other than `*`
 -}
-binaryField :: ContentType -> [ContentType] -> Bool -> ReadRequest -> Either Response (Maybe FieldName)
-binaryField ct rawContentTypes isScalarProc readReq
-  | isScalarProc = Right Nothing
+binaryField :: ContentType -> [ContentType] -> Bool -> Target -> ReadRequest -> Either Response (Maybe FieldName)
+binaryField ct rawContentTypes hasSelect target readReq
+  | isTargetProc && not hasSelect = Right Nothing
   | ct `elem` rawContentTypes =
       let fieldName = headMay fldNames in
       if length fldNames == 1 && fieldName /= Just "*"
@@ -400,6 +400,10 @@ binaryField ct rawContentTypes isScalarProc readReq
   | otherwise = Right Nothing
   where
     fldNames = fstFieldNames readReq
+    isTargetProc =
+      case target of
+        TargetProc{} -> True
+        _            -> False
 
 locationH :: TableName -> [BS.ByteString] -> Header
 locationH tName fields =

src/PostgREST/Private/QueryFragment.hs

@@ -84,7 +84,7 @@ 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 "
+asJsonSingleF = "coalesce(string_agg(to_json(_postgrest_t)::text, ','), '')::character varying "
 
 asBinaryF :: FieldName -> SqlFragment
 asBinaryF fieldName = "coalesce(string_agg(_postgrest_t." <> pgFmtIdent fieldName <> ", ''), '')"

src/PostgREST/QueryBuilder.hs

@@ -120,8 +120,8 @@ mutateRequestToQuery (Delete mainQi logicForest returnings) =
     returningF mainQi returnings
     ]
 
-requestToCallProcQuery :: QualifiedIdentifier -> [PgArg] -> Bool -> Maybe PreferParameters -> [FieldName] -> SqlQuery
-requestToCallProcQuery qi pgArgs returnsScalar preferParams returnings =
+requestToCallProcQuery :: QualifiedIdentifier -> [PgArg] -> Maybe PreferParameters -> [FieldName] -> SqlQuery
+requestToCallProcQuery qi pgArgs preferParams returnings =
   BS.unwords [
     "WITH",
     argsCTE,
@@ -153,23 +153,18 @@ requestToCallProcQuery qi pgArgs returnsScalar preferParams returnings =
     sourceBody :: SqlFragment
     sourceBody
       | paramsAsMultipleObjects =
-          if returnsScalar
-            then "SELECT " <> callIt <> " AS pgrst_scalar FROM pgrst_args"
-            else BS.unwords [ "SELECT pgrst_lat_args.*"
-                         , "FROM pgrst_args,"
-                         , "LATERAL ( SELECT " <> returned_columns <> " FROM " <> callIt <> " ) pgrst_lat_args" ]
-      | otherwise =
-          if returnsScalar
-            then "SELECT " <> callIt <> " AS pgrst_scalar"
-            else "SELECT " <> returned_columns <> " FROM " <> callIt
+          BS.unwords [ "SELECT pgrst_lat_args.*",
+                       "FROM pgrst_args,",
+                       "LATERAL ( SELECT " <> returned_columns <> " FROM " <> callIt <> " ) pgrst_lat_args" ]
+      | otherwise = "SELECT " <> returned_columns <> " FROM " <> callIt
 
     callIt :: SqlFragment
-    callIt = fromQi qi <> "(" <> args <> ")"
+    callIt = fromQi qi <> "(" <> args <> ") AS pgrst_scalar"
 
     returned_columns :: SqlFragment
     returned_columns
       | null returnings = "*"
-      | otherwise       = BS.intercalate ", " (pgFmtColumn (QualifiedIdentifier mempty $ qiName qi) <$> returnings)
+      | otherwise       = BS.intercalate ", " (pgFmtColumn (QualifiedIdentifier mempty "pgrst_scalar") <$> returnings)
 
 
 -- | SQL query meant for COUNTing the root node of the Tree.

src/PostgREST/Statements.hs

@@ -126,36 +126,38 @@ standardRow = (,,,,,) <$> nullableColumn HD.int8 <*> column HD.int8
 
 type ProcResults = (Maybe Int64, Int64, ByteString, Either SimpleError [GucHeader], Either SimpleError (Maybe Status))
 
-callProcStatement :: Bool -> SqlQuery -> SqlQuery -> SqlQuery -> Bool ->
+callProcStatement :: SqlQuery -> SqlQuery -> SqlQuery -> Bool ->
                      Bool -> Bool -> Bool -> Bool -> Maybe FieldName -> PgVersion ->
                      H.Statement ByteString ProcResults
-callProcStatement returnsScalar callProcQuery selectQuery countQuery countTotal isSingle asCsv asBinary multObjects binaryField pgVer =
+callProcStatement callProcQuery selectQuery countQuery countTotal isSingle asCsv asBinary multObjects binaryField pgVer =
   H.Statement sql (param HE.unknown) decodeProc True
   where
     sql = [qc|
-      WITH {sourceCTEName} AS ({callProcQuery})
+      WITH {sourceCTEName} AS ({callProcQuery}),
+      pgrst_select AS ({selectQuery}),
+      pgrst_to_json AS (SELECT to_jsonb(pgrst_select) AS pgrst_json FROM pgrst_select)
       {countCTEF}
       SELECT
         {countResultF} AS total_result_set,
         pg_catalog.count(_postgrest_t) AS page_total,
         {bodyF} AS body,
         {responseHeadersF pgVer} AS response_headers,
         {responseStatusF pgVer} AS response_status
-      FROM ({selectQuery}) _postgrest_t;|]
+      FROM {selectF} AS _postgrest_t;|]
 
     (countCTEF, countResultF) = countF countQuery countTotal
 
     bodyF
-     | returnsScalar = scalarBodyF
-     | isSingle     = asJsonSingleF
-     | 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"
+     | asBinary           = asBinaryF $ fromMaybe "pgrst_scalar" binaryField
+     | isSingle
+       && not multObjects = asJsonSingleF
+     | asCsv              = asCsvF
+     | otherwise          = asJsonF
+
+    selectF :: SqlFragment
+    selectF
+     | asCsv || asBinary = "pgrst_select"
+     | otherwise         = "(SELECT CASE WHEN pgrst_json ? 'pgrst_scalar' THEN pgrst_json->'pgrst_scalar' ELSE pgrst_json END AS _postgrest_t FROM pgrst_to_json)"
 
     decodeProc :: HD.Result ProcResults
     decodeProc =

src/PostgREST/Types.hs

@@ -191,10 +191,10 @@ specifiedProcArgs :: S.Set FieldName -> ProcDescription -> [PgArg]
 specifiedProcArgs keys proc =
   (\k -> fromMaybe (PgArg k "text" True False) (find ((==) k . pgaName) (pdArgs proc))) <$> S.toList keys
 
-procReturnsScalar :: ProcDescription -> Bool
-procReturnsScalar proc = case proc of
-  ProcDescription{pdReturnType = (Single (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

test/Feature/QuerySpec.hs

@@ -821,14 +821,16 @@ 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]
-        }
+          [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` 406
+        `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` 406
+        `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") ""