refactor: change CallPlan returnings to a Set instead of a List

This commit is contained in:
Taimoor Zaeem
2025-04-26 10:02:30 -05:00
committed by Steve Chavez
parent 01432ce963
commit cd5a611a1a
4 changed files with 12 additions and 10 deletions
+7 -7
View File
@@ -995,7 +995,7 @@ mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{
returnings = returnings =
if preferRepresentation == Just None || isNothing preferRepresentation if preferRepresentation == Just None || isNothing preferRepresentation
then [] then []
else inferColsEmbedNeeds readReq pkCols else S.toList $ inferColsEmbedNeeds readReq pkCols
-- TODO: remove fromJust by refactoring later -- TODO: remove fromJust by refactoring later
-- we can use fromJust, we have already looked up the table before building mutatePlan -- we can use fromJust, we have already looked up the table before building mutatePlan
tbl = fromJust $ HM.lookup qi dbTables tbl = fromJust $ HM.lookup qi dbTables
@@ -1029,21 +1029,21 @@ callPlan proc ApiRequest{} paramKeys args readReq = FunctionCall {
prms -> KeyParams $ specifiedParams prms prms -> KeyParams $ specifiedParams prms
-- | Infers the columns needed for an embed to be successful after a mutation or a function call. -- | 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 inferColsEmbedNeeds (Node ReadPlan{select} forest) pkCols
-- if * is part of the select, we must not add pk or fk columns manually - -- if * is part of the select, we must not add pk or fk columns manually -
-- otherwise those would be selected and output twice -- otherwise those would be selected and output twice
| "*" `elem` fldNames = ["*"] | "*" `S.member` fldNames = S.singleton "*"
| otherwise = returnings | otherwise = returnings
where where
fldNames = cfName . csField <$> select fldNames = S.fromList $ cfName . csField <$> select
-- Without fkCols, when a mutatePlan to -- Without fkCols, when a mutatePlan to
-- /projects?select=name,clients(name) occurs, the RETURNING SQL part would -- /projects?select=name,clients(name) occurs, the RETURNING SQL part would
-- be `RETURNING name`(see QueryBuilder). This would make the embedding -- be `RETURNING name`(see QueryBuilder). This would make the embedding
-- fail because the following JOIN would need the "client_id" column from -- fail because the following JOIN would need the "client_id" column from
-- projects. So this adds the foreign key columns to ensure the embedding -- projects. So this adds the foreign key columns to ensure the embedding
-- succeeds, result would be `RETURNING name, client_id`. -- 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}} _ -> Node ReadPlan{relToParent=Just Relationship{relCardinality=O2M _ cols}} _ ->
Just $ fst <$> cols Just $ fst <$> cols
Node ReadPlan{relToParent=Just Relationship{relCardinality=M2O _ cols}} _ -> Node ReadPlan{relToParent=Just Relationship{relCardinality=M2O _ cols}} _ ->
@@ -1070,8 +1070,8 @@ inferColsEmbedNeeds (Node ReadPlan{select} forest) pkCols
-- INSERT/POST -- INSERT/POST
returnings = returnings =
if not hasComputedRel if not hasComputedRel
then S.toList . S.fromList $ fldNames ++ fkCols ++ pkCols then fldNames <> fkCols <> S.fromList pkCols
else ["*"] -- on computed relationships we cannot know the required columns for an embedding to succeed, so we just return all 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 -- Traditional filters(e.g. id=eq.1) are added as root nodes of the LogicTree
-- they are later concatenated with AND in the QueryBuilder -- they are later concatenated with AND in the QueryBuilder
+1 -1
View File
@@ -25,7 +25,7 @@ data CallPlan = FunctionCall
, funCScalar :: Bool , funCScalar :: Bool
, funCSetOfScalar :: Bool , funCSetOfScalar :: Bool
, funCRetCompositeAlias :: Bool , funCRetCompositeAlias :: Bool
, funCReturning :: [FieldName] , funCReturning :: Set FieldName
} }
data CallParams data CallParams
+1
View File
@@ -30,6 +30,7 @@ data JoinCondition =
(QualifiedIdentifier, FieldName) (QualifiedIdentifier, FieldName)
deriving (Eq, Show) deriving (Eq, Show)
-- TODO: Enforce uniqueness of columns by changing to a Set instead of a List where applicable
data ReadPlan = ReadPlan data ReadPlan = ReadPlan
{ select :: [CoercibleSelectField] { select :: [CoercibleSelectField]
, from :: QualifiedIdentifier , from :: QualifiedIdentifier
+2 -1
View File
@@ -20,6 +20,7 @@ module PostgREST.Query.QueryBuilder
import qualified Data.Aeson as JSON import qualified Data.Aeson as JSON
import qualified Data.ByteString.Char8 as BS import qualified Data.ByteString.Char8 as BS
import qualified Data.HashMap.Strict as HM import qualified Data.HashMap.Strict as HM
import qualified Data.Set as S
import qualified Hasql.DynamicStatements.Snippet as SQL import qualified Hasql.DynamicStatements.Snippet as SQL
import qualified Hasql.Encoders as HE import qualified Hasql.Encoders as HE
@@ -213,7 +214,7 @@ callPlanToQuery (FunctionCall qi params arguments returnsScalar returnsSetOfScal
returnedColumns :: SQL.Snippet returnedColumns :: SQL.Snippet
returnedColumns returnedColumns
| null returnings = "*" | 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. -- | 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. -- It only takes WHERE into account and doesn't include LIMIT/OFFSET because it would reduce the COUNT.