From cd5a611a1af023a067e67871a4955aecf0e15e1b Mon Sep 17 00:00:00 2001
From: Taimoor Zaeem <taimoorzaeem@gmail.com>
Date: Sat, 26 Apr 2025 14:57:34 +0500
Subject: [PATCH] refactor: change CallPlan returnings to a Set instead of a
 List

---
 src/PostgREST/Plan.hs               | 16 ++++++++--------
 src/PostgREST/Plan/CallPlan.hs      |  2 +-
 src/PostgREST/Plan/ReadPlan.hs      |  1 +
 src/PostgREST/Query/QueryBuilder.hs |  3 ++-
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs
index c7ad19c84..e5c205ffa 100644
--- a/src/PostgREST/Plan.hs
+++ b/src/PostgREST/Plan.hs
@@ -995,7 +995,7 @@ mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{
     returnings =
       if preferRepresentation == Just None || isNothing preferRepresentation
         then []
-        else inferColsEmbedNeeds readReq pkCols
+        else S.toList $ inferColsEmbedNeeds readReq pkCols
     -- TODO: remove fromJust by refactoring later
     -- we can use fromJust, we have already looked up the table before building mutatePlan
     tbl = fromJust $ HM.lookup qi dbTables
@@ -1029,21 +1029,21 @@ callPlan proc ApiRequest{} paramKeys args readReq = FunctionCall {
       prms  -> KeyParams $ specifiedParams prms
 
 -- | Infers the columns needed for an embed to be successful after a mutation or a function call.
-inferColsEmbedNeeds :: ReadPlanTree -> [FieldName] -> [FieldName]
+inferColsEmbedNeeds :: ReadPlanTree -> [FieldName] -> S.Set FieldName
 inferColsEmbedNeeds (Node ReadPlan{select} forest) pkCols
   -- if * is part of the select, we must not add pk or fk columns manually -
   -- otherwise those would be selected and output twice
-  | "*" `elem` fldNames = ["*"]
-  | otherwise           = returnings
+  | "*" `S.member` fldNames = S.singleton "*"
+  | otherwise               = returnings
   where
-    fldNames = cfName . csField <$> select
+    fldNames = S.fromList $ cfName . csField <$> select
     -- Without fkCols, when a mutatePlan to
     -- /projects?select=name,clients(name) occurs, the RETURNING SQL part would
     -- be `RETURNING name`(see QueryBuilder).  This would make the embedding
     -- fail because the following JOIN would need the "client_id" column from
     -- projects.  So this adds the foreign key columns to ensure the embedding
     -- succeeds, result would be `RETURNING name, client_id`.
-    fkCols = concat $ mapMaybe (\case
+    fkCols = S.fromList $ concat $ mapMaybe (\case
         Node ReadPlan{relToParent=Just Relationship{relCardinality=O2M _ cols}} _ ->
           Just $ fst <$> cols
         Node ReadPlan{relToParent=Just Relationship{relCardinality=M2O _ cols}} _ ->
@@ -1070,8 +1070,8 @@ inferColsEmbedNeeds (Node ReadPlan{select} forest) pkCols
     -- INSERT/POST
     returnings =
       if not hasComputedRel
-        then S.toList . S.fromList $ fldNames ++ fkCols ++ pkCols
-        else ["*"] -- on computed relationships we cannot know the required columns for an embedding to succeed, so we just return all
+        then fldNames <> fkCols <> S.fromList pkCols
+        else S.singleton "*" -- on computed relationships we cannot know the required columns for an embedding to succeed, so we just return all
 
 -- Traditional filters(e.g. id=eq.1) are added as root nodes of the LogicTree
 -- they are later concatenated with AND in the QueryBuilder
diff --git a/src/PostgREST/Plan/CallPlan.hs b/src/PostgREST/Plan/CallPlan.hs
index 32ef34c35..47b06a2c6 100644
--- a/src/PostgREST/Plan/CallPlan.hs
+++ b/src/PostgREST/Plan/CallPlan.hs
@@ -25,7 +25,7 @@ data CallPlan = FunctionCall
   , funCScalar            :: Bool
   , funCSetOfScalar       :: Bool
   , funCRetCompositeAlias :: Bool
-  , funCReturning         :: [FieldName]
+  , funCReturning         :: Set FieldName
   }
 
 data CallParams
diff --git a/src/PostgREST/Plan/ReadPlan.hs b/src/PostgREST/Plan/ReadPlan.hs
index 5563fbec8..22bd7d62e 100644
--- a/src/PostgREST/Plan/ReadPlan.hs
+++ b/src/PostgREST/Plan/ReadPlan.hs
@@ -30,6 +30,7 @@ data JoinCondition =
     (QualifiedIdentifier, FieldName)
   deriving (Eq, Show)
 
+-- TODO: Enforce uniqueness of columns by changing to a Set instead of a List where applicable
 data ReadPlan = ReadPlan
   { select       :: [CoercibleSelectField]
   , from         :: QualifiedIdentifier
diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs
index 33193184c..cc108b388 100644
--- a/src/PostgREST/Query/QueryBuilder.hs
+++ b/src/PostgREST/Query/QueryBuilder.hs
@@ -20,6 +20,7 @@ module PostgREST.Query.QueryBuilder
 import qualified Data.Aeson                      as JSON
 import qualified Data.ByteString.Char8           as BS
 import qualified Data.HashMap.Strict             as HM
+import qualified Data.Set                        as S
 import qualified Hasql.DynamicStatements.Snippet as SQL
 import qualified Hasql.Encoders                  as HE
 
@@ -213,7 +214,7 @@ callPlanToQuery (FunctionCall qi params arguments returnsScalar returnsSetOfScal
     returnedColumns :: SQL.Snippet
     returnedColumns
       | null returnings = "*"
-      | otherwise       = intercalateSnippet ", " (pgFmtColumn (QualifiedIdentifier mempty "pgrst_call") <$> returnings)
+      | otherwise       = intercalateSnippet ", " (pgFmtColumn (QualifiedIdentifier mempty "pgrst_call") <$> S.toList returnings)
 
 -- | SQL query meant for COUNTing the root node of the Tree.
 -- It only takes WHERE into account and doesn't include LIMIT/OFFSET because it would reduce the COUNT.