Skip to content

Commit

Permalink
fix: HEAD unnecessarily executing aggregates
Browse files Browse the repository at this point in the history
  • Loading branch information
steve-chavez committed Jul 10, 2023
1 parent 905fcb0 commit e332f03
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- #2821, Fix OPTIONS not accepting all available media types - @steve-chavez
- #2834, Fix compilation on Ubuntu by being compatible with GHC 9.0.2 - @steve-chavez
- #2840, Fix `Prefer: missing=default` with DOMAIN default values - @steve-chavez
- #2849, Fix HEAD unnecessarily executing aggregates - @steve-chavez

## [11.1.0] - 2023-06-07

Expand Down
6 changes: 0 additions & 6 deletions src/PostgREST/MediaType.hs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ module PostgREST.MediaType
, toContentType
, toMime
, decodeMediaType
, getMediaType
) where

import qualified Data.ByteString as BS
Expand Down Expand Up @@ -143,8 +142,3 @@ decodeMediaType mt =
[PlanSettings | inOpts "settings"] ++
[PlanBuffers | inOpts "buffers" ] ++
[PlanWAL | inOpts "wal" ]

getMediaType :: MediaType -> MediaType
getMediaType mt = case mt of
MTPlan mType _ _ -> mType
other -> other
24 changes: 15 additions & 9 deletions src/PostgREST/Plan.hs
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ wrappedReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest
wrappedReadPlan identifier conf sCache apiRequest = do
rPlan <- readPlan identifier conf sCache apiRequest
binField <- mapLeft ApiRequestError $ binaryField conf (iAcceptMediaType apiRequest) Nothing rPlan
return $ WrappedReadPlan rPlan SQL.Read $ mediaToAggregate (iAcceptMediaType apiRequest) binField Nothing
return $ WrappedReadPlan rPlan SQL.Read $ mediaToAggregate (iAcceptMediaType apiRequest) binField apiRequest

mutateReadPlan :: Mutation -> ApiRequest -> QualifiedIdentifier -> AppConfig -> SchemaCache -> Either Error MutateReadPlan
mutateReadPlan mutation apiRequest@ApiRequest{iPreferences=Preferences{preferRepresentation}} identifier conf sCache = do
mutateReadPlan mutation apiRequest identifier conf sCache = do
rPlan <- readPlan identifier conf sCache apiRequest
binField <- mapLeft ApiRequestError $ binaryField conf (iAcceptMediaType apiRequest) Nothing rPlan
mPlan <- mutatePlan mutation identifier apiRequest sCache rPlan
return $ MutateReadPlan rPlan mPlan SQL.Write $ mediaToAggregate (iAcceptMediaType apiRequest) binField (Just preferRepresentation)
return $ MutateReadPlan rPlan mPlan SQL.Write $ mediaToAggregate (iAcceptMediaType apiRequest) binField apiRequest

callReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> InvokeMethod -> Either Error CallReadPlan
callReadPlan identifier conf sCache apiRequest invMethod = do
Expand All @@ -144,7 +144,7 @@ callReadPlan identifier conf sCache apiRequest invMethod = do
(InvPost, Routine.Volatile) -> SQL.Write
cPlan = callPlan proc apiRequest paramKeys args rPlan
binField <- mapLeft ApiRequestError $ binaryField conf (iAcceptMediaType apiRequest) (Just proc) rPlan
return $ CallReadPlan rPlan cPlan txMode proc $ mediaToAggregate (iAcceptMediaType apiRequest) binField Nothing
return $ CallReadPlan rPlan cPlan txMode proc $ mediaToAggregate (iAcceptMediaType apiRequest) binField apiRequest
where
Preferences{..} = iPreferences apiRequest
qsParams' = QueryParams.qsParams (iQueryParams apiRequest)
Expand Down Expand Up @@ -839,10 +839,10 @@ binaryField AppConfig{configRawMediaTypes} acceptMediaType proc rpTree
fstFieldName (Node ReadPlan{select=[(CoercibleField{cfName=fld, cfJsonPath=[]}, _, _)]} []) = Just fld
fstFieldName _ = Nothing

mediaToAggregate :: MediaType -> Maybe FieldName -> Maybe PreferRepresentation -> ResultAggregate
mediaToAggregate mt binField rep =
if rep == Just HeadersOnly || rep == Just None
then NoAgg

mediaToAggregate :: MediaType -> Maybe FieldName -> ApiRequest -> ResultAggregate
mediaToAggregate mt binField apiReq@ApiRequest{iAction=act, iPreferences=Preferences{preferRepresentation=rep}} =
if noAgg then NoAgg
else case mt of
MTApplicationJSON -> BuiltinAggJson
MTSingularJSON -> BuiltinAggSingleJson
Expand All @@ -861,4 +861,10 @@ mediaToAggregate mt binField rep =
-- Doing `Accept: application/vnd.pgrst.plan; for="application/vnd.pgrst.plan"` doesn't make sense, so we just empty the body.
-- TODO: fail instead to be more strict
MTPlan (MTPlan{}) _ _ -> NoAgg
MTPlan media _ _ -> mediaToAggregate media binField rep
MTPlan media _ _ -> mediaToAggregate media binField apiReq
where
noAgg = case act of
ActionMutate _ -> rep == HeadersOnly || rep == None
ActionRead _isHead -> _isHead -- no need for an aggregate on HEAD https://github.com/PostgREST/postgrest/issues/2849
ActionInvoke invMethod -> invMethod == InvHead
_ -> False

0 comments on commit e332f03

Please sign in to comment.