Skip to content

feat: [WIP] parameters tracked snippets, and application/json+sql response #4012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat: [WIP] parameters tracked snippets, and application/json+sql response #4012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a8500be
init raw sql return type on read
fauh45 Apr 10, 2025
ae3d4ad
fix: add MTApplicationSQL on initialMediaHandlers
fauh45 Apr 10, 2025
cc2901d
feat: return sql on mutate as well
fauh45 Apr 10, 2025
7535bb2
test: add raw sql query test
fauh45 Apr 10, 2025
bf5aaa8
fix: should be application/sql instead of json
fauh45 Apr 10, 2025
3dd222d
feat: add tracked snippets and params response
fauh45 Apr 13, 2025
eb98b1e
chore: fix accidental formatting
fauh45 Apr 13, 2025
d378f2d
chore: update inline comments to better describe TrackedSnippet
fauh45 Apr 13, 2025
85d96e5
fix: move to application/json+sql to make it more clear on the return…
fauh45 Apr 13, 2025
ab98656
fix: change raw sql response json parameters name more descriptive
fauh45 Apr 13, 2025
ff87f23
test: add testing for application/json+sql request
fauh45 Apr 13, 2025
68472c7
fix: badly formatted qi
fauh45 Apr 13, 2025
2243b97
chore: add changelog for tracked snippet and application/json+sql res…
fauh45 Apr 14, 2025
ea338e1
test: add MTApplicationJSONSQL to dbMediaHanlders on CLI
fauh45 Apr 16, 2025
8a2f56b
chore: fix formatting
fauh45 Apr 16, 2025
d005ed3
fix: remove unused functions
fauh45 Apr 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
+ The selected columns in the embedded resources are aggregated into arrays
+ Aggregates are not supported
- #2967, Add `Proxy-Status` header for better error response - @taimoorzaeem
- #4012, Add parameters tracked Snippets to allow for raw sql response with templated parameters, and application/json+sql to return the raw sql - @fauh45

### Fixed

Expand Down
1 change: 1 addition & 0 deletions postgrest.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ test-suite spec
Feature.Query.QuerySpec
Feature.Query.RangeSpec
Feature.Query.RawOutputTypesSpec
Feature.Query.RawSQLSpec
Feature.Query.RelatedQueriesSpec
Feature.Query.RpcSpec
Feature.Query.ServerTimingSpec
Expand Down
3 changes: 3 additions & 0 deletions src/PostgREST/MediaType.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import Protolude
-- | Enumeration of currently supported media types
data MediaType
= MTApplicationJSON
| MTApplicationJSONSQL
| MTGeoJSON
| MTTextCSV
| MTTextPlain
Expand Down Expand Up @@ -65,6 +66,7 @@ toContentType ct = (hContentType, toMime ct <> charset)
-- | Convert from MediaType to a ByteString representing the mime type
toMime :: MediaType -> ByteString
toMime MTApplicationJSON = "application/json"
toMime MTApplicationJSONSQL = "application/json+sql"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

application/json suffixes have to be registered on IANA to be valid (ref).

Unregistered suffixes should not be used

So we would need a vendored media type. I suggest application/vnd.pgrst.sql+json.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since +json is a registered suffix, would application/sql+json work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

application/sql+json

IIUC, we would have to register the +json to the SQL media type if we want to use it like that.

That being said, I like that media type too and it doesn't seem "potentially damaging", unlike adding a suffix to application/json.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like the idea of vendoring the media type, though prefixing sql makes it more clear though IMO. Just to be safe, I'll probably go with the vendored media type for now

toMime MTVndArrayJSONStrip = "application/vnd.pgrst.array+json;nulls=stripped"
toMime MTGeoJSON = "application/geo+json"
toMime MTTextCSV = "text/csv"
Expand Down Expand Up @@ -133,6 +135,7 @@ decodeMediaType mt = decodeMediaType' $ decodeLatin1 mt
decodeMediaType' mt' =
case (T.toLower mainType, T.toLower subType, params) of
("application", "json", _) -> MTApplicationJSON
("application", "json+sql", _) -> MTApplicationJSONSQL
("application", "geo+json", _) -> MTGeoJSON
("text", "csv", _) -> MTTextCSV
("text", "plain", _) -> MTTextPlain
Expand Down
1 change: 1 addition & 0 deletions src/PostgREST/Plan.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,7 @@ negotiateContent conf ApiRequest{iAction=act, iPreferences=Preferences{preferRep
-- TODO: despite no aggregate, these are responding with a Content-Type, which is not correct.
(ActDb (ActRelationRead _ True), Just (_, mt)) -> Right (NoAgg, mt)
(ActDb (ActRoutine _ (InvRead True)), Just (_, mt)) -> Right (NoAgg, mt)
(_, Just (_, MTApplicationJSONSQL)) -> Right (NoAgg, MTApplicationJSONSQL)
Copy link
Member

@steve-chavez steve-chavez Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This media type should be opt in (it reveals internal details that should only be available for debugging purposes), I suggest reusing the https://docs.postgrest.org/en/v12/references/configuration.html#db-plan-enabled (configDbPlanEnabled) for this. See how it's done on:

postgrest/src/PostgREST/Plan.hs

Lines 1098 to 1104 in c732591

m@(MTVndPlan (MTVndSingularJSON strip) _ _) -> mtPlanToNothing $ Just (BuiltinAggSingleJson strip, m)
m@(MTVndPlan MTVndArrayJSONStrip _ _) -> mtPlanToNothing $ Just (BuiltinAggArrayJsonStrip, m)
-- TODO the plan should have its own MediaHandler instead of relying on MediaType
m@(MTVndPlan mType _ _) -> mtPlanToNothing $ (,) <$> (fst <$> lookupHandler mType) <*> pure m
-- all the other media types can be overridden
x -> lookupHandler x
mtPlanToNothing x = if configDbPlanEnabled conf then x else Nothing -- don't find anything if the plan media type is not allowed

It's a bit messy now unfortunately.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah so in a way on the Plan there should be a check to allow it if the configDbPlanEnabled? Honestly I totally understand that, enabling this does open up lots of internal details

(_, Just (x, mt)) -> Right (x, mt)
where
firstAcceptedPick = listToMaybe $ mapMaybe matchMT accepts -- If there are multiple accepted media types, pick the first. This is usual in content negotiation.
Expand Down
66 changes: 53 additions & 13 deletions src/PostgREST/Query.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import qualified Data.HashMap.Strict as HM
import qualified Data.Set as S
import qualified Hasql.Decoders as HD
import qualified Hasql.DynamicStatements.Snippet as SQL (Snippet)
import qualified Hasql.DynamicStatements.Statement as SQL
import qualified Hasql.Session as SQL (Session)
import qualified Hasql.Transaction as SQL
Expand Down Expand Up @@ -49,11 +48,13 @@
InfoPlan (..),
InspectPlan (..))
import PostgREST.Plan.MutatePlan (MutatePlan (..))
import PostgREST.Query.SqlFragment (escapeIdentList, fromQi,
intercalateSnippet,
import PostgREST.Query.SqlFragment (TrackedSnippet,
escapeIdentList, fromQi,
intercalateSnippet, rawSQL,
setConfigWithConstantName,
setConfigWithConstantNameJSON,
setConfigWithDynamicName)
setConfigWithDynamicName,
toSnippet)
import PostgREST.Query.Statements (ResultSet (..))
import PostgREST.SchemaCache (SchemaCache (..))
import PostgREST.SchemaCache.Identifiers (QualifiedIdentifier (..))
Expand All @@ -79,6 +80,7 @@
| DbCallResult CallReadPlan ResultSet
| MaybeDbResult InspectPlan (Maybe (TablesMap, RoutineMap, Maybe Text))
| NoDbResult InfoPlan
| RawSQLResult ByteString [Maybe ByteString]

query :: AppConfig -> AuthResult -> ApiRequest -> ActionPlan -> SchemaCache -> PgVersion -> Query
query _ _ _ (NoDb x) _ _ = NoDbQuery $ NoDbResult x
Expand Down Expand Up @@ -109,11 +111,31 @@

-- TODO: Generate the Hasql Statement in a diferent module after the OpenAPI functionality is removed
actionQuery :: DbActionPlan -> AppConfig -> ApiRequest -> PgVersion -> SchemaCache -> (DbHandler QueryResult, ByteString)
-- NOTE: Test handling if wrMedia is equal to MTApplicationSQL, returns RawSQLResult which will not be queried, instead directly returned
actionQuery (DbCrud WrappedReadPlan{wrMedia = MTApplicationJSONSQL, ..}) AppConfig{..} ApiRequest{iPreferences=Preferences{..}} _ _ =
(mainActionQuery, mainSQLQuery)
where
countQuery = QueryBuilder.readPlanToCountQuery wrReadPlan

Check warning on line 118 in src/PostgREST/Query.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Query.hs#L118 

Added line #L118 was not covered by tests
(_, mainSQLQuery, params) = Statements.prepareRead
(QueryBuilder.readPlanToQuery wrReadPlan)
(if preferCount == Just EstimatedCount then

Check warning on line 121 in src/PostgREST/Query.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Query.hs#L121 

Added line #L121 was not covered by tests
-- LIMIT maxRows + 1 so we can determine below that maxRows was surpassed
QueryBuilder.limitedQuery countQuery ((+ 1) <$> configDbMaxRows)

Check warning on line 123 in src/PostgREST/Query.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Query.hs#L123 

Added line #L123 was not covered by tests
else
countQuery

Check warning on line 125 in src/PostgREST/Query.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Query.hs#L125 

Added line #L125 was not covered by tests
)
(shouldCount preferCount)
MTApplicationJSONSQL
wrHandler
configDbPreparedStatements

Check warning on line 130 in src/PostgREST/Query.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Query.hs#L130 

Added line #L130 was not covered by tests
mainActionQuery = do
pure $ RawSQLResult mainSQLQuery params

actionQuery (DbCrud plan@WrappedReadPlan{..}) conf@AppConfig{..} apiReq@ApiRequest{iPreferences=Preferences{..}} _ _ =
(mainActionQuery, mainSQLQuery)
where
countQuery = QueryBuilder.readPlanToCountQuery wrReadPlan
(result, mainSQLQuery) = Statements.prepareRead
(result, mainSQLQuery, _) = Statements.prepareRead
(QueryBuilder.readPlanToQuery wrReadPlan)
(if preferCount == Just EstimatedCount then
-- LIMIT maxRows + 1 so we can determine below that maxRows was surpassed
Expand All @@ -131,11 +153,29 @@
optionalRollback conf apiReq
DbCrudResult plan <$> resultSetWTotal conf apiReq resultSet countQuery

actionQuery (DbCrud MutateReadPlan{mrMedia = MTApplicationJSONSQL, ..}) AppConfig{..} ApiRequest{iPreferences=Preferences{..}} _ _ =
(mainActionQuery, mainSQLQuery)
where
(isPut, isInsert, pkCols) = case mrMutatePlan of {Insert{where_,insPkCols} -> ((not . null) where_, True, insPkCols); _ -> (False,False, mempty);}
(_, mainSQLQuery, params) = Statements.prepareWrite
(QueryBuilder.readPlanToQuery mrReadPlan)

Check warning on line 161 in src/PostgREST/Query.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Query.hs#L161 

Added line #L161 was not covered by tests
(QueryBuilder.mutatePlanToQuery mrMutatePlan)
isInsert
isPut
MTApplicationJSONSQL
mrHandler
preferRepresentation
preferResolution
pkCols
configDbPreparedStatements

Check warning on line 170 in src/PostgREST/Query.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Query.hs#L169-L170 

Added lines #L169 - L170 were not covered by tests
mainActionQuery = do
pure $ RawSQLResult mainSQLQuery params

actionQuery (DbCrud plan@MutateReadPlan{..}) conf@AppConfig{..} apiReq@ApiRequest{iPreferences=Preferences{..}} _ _ =
(mainActionQuery, mainSQLQuery)
where
(isPut, isInsert, pkCols) = case mrMutatePlan of {Insert{where_,insPkCols} -> ((not . null) where_, True, insPkCols); _ -> (False,False, mempty);}
(result, mainSQLQuery) = Statements.prepareWrite
(result, mainSQLQuery, _) = Statements.prepareWrite
(QueryBuilder.readPlanToQuery mrReadPlan)
(QueryBuilder.mutatePlanToQuery mrMutatePlan)
isInsert
Expand All @@ -151,12 +191,12 @@
failNotSingular mrMedia resultSet
MutationUpdate -> do
failNotSingular mrMedia resultSet
failExceedsMaxAffectedPref (preferMaxAffected,preferHandling) resultSet
failExceedsMaxAffectedPref (preferMaxAffected, preferHandling) resultSet
MutationSingleUpsert -> do
failPut resultSet
MutationDelete -> do
failNotSingular mrMedia resultSet
failExceedsMaxAffectedPref (preferMaxAffected,preferHandling) resultSet
failExceedsMaxAffectedPref (preferMaxAffected, preferHandling) resultSet
mainActionQuery = do
resultSet <- lift $ SQL.statement mempty result
failMutation resultSet
Expand All @@ -166,7 +206,7 @@
actionQuery (DbCall plan@CallReadPlan{..}) conf@AppConfig{..} apiReq@ApiRequest{iPreferences=Preferences{..}} pgVer _ =
(mainActionQuery, mainSQLQuery)
where
(result, mainSQLQuery) = Statements.prepareCall
(result, mainSQLQuery, _) = Statements.prepareCall
crProc
(QueryBuilder.callPlanToQuery crCallPlan pgVer)
(QueryBuilder.readPlanToQuery crReadPlan)
Expand All @@ -179,7 +219,7 @@
resultSet <- lift $ SQL.statement mempty result
optionalRollback conf apiReq
failNotSingular crMedia resultSet
failExceedsMaxAffectedPref (preferMaxAffected,preferHandling) resultSet
failExceedsMaxAffectedPref (preferMaxAffected, preferHandling) resultSet
pure $ DbCallResult plan resultSet

actionQuery (MaybeDb plan@InspectPlan{ipSchema=tSchema}) AppConfig{..} _ _ sCache =
Expand Down Expand Up @@ -213,7 +253,7 @@
lift SQL.condemn
throwError $ Error.ApiRequestError Error.PutMatchingPkError

resultSetWTotal :: AppConfig -> ApiRequest -> ResultSet -> SQL.Snippet -> DbHandler ResultSet
resultSetWTotal :: AppConfig -> ApiRequest -> ResultSet -> TrackedSnippet -> DbHandler ResultSet
resultSetWTotal _ _ rs@RSPlan{} _ = return rs
resultSetWTotal AppConfig{..} ApiRequest{iPreferences=Preferences{..}} rs@RSStandard{rsTableTotal=tableTotal} countQuery =
case preferCount of
Expand Down Expand Up @@ -270,7 +310,7 @@
SQL.statement mempty $ SQL.dynamicallyParameterized
-- To ensure `GRANT SET ON PARAMETER <superuser_setting> TO authenticator` works, the role settings must be set before the impersonated role.
-- Otherwise the GRANT SET would have to be applied to the impersonated role. See https://github.com/PostgREST/postgrest/issues/3045
("select " <> intercalateSnippet ", " (searchPathSql : roleSettingsSql ++ roleSql ++ claimsSql ++ [methodSql, pathSql] ++ headersSql ++ cookiesSql ++ timezoneSql ++ funcSettingsSql ++ appSettingsSql))
(toSnippet (rawSQL "select " <> intercalateSnippet ", " (searchPathSql : roleSettingsSql ++ roleSql ++ claimsSql ++ [methodSql, pathSql] ++ headersSql ++ cookiesSql ++ timezoneSql ++ funcSettingsSql ++ appSettingsSql)))
HD.noResult configDbPreparedStatements
where
methodSql = setConfigWithConstantName ("request.method", iMethod)
Expand All @@ -295,7 +335,7 @@
runPreReq conf = lift $ traverse_ (SQL.statement mempty . stmt) (configDbPreRequest conf)
where
stmt req = SQL.dynamicallyParameterized
("select " <> fromQi req <> "()")
(toSnippet (rawSQL "select " <> fromQi req <> rawSQL "()"))
HD.noResult
(configDbPreparedStatements conf)

Expand Down
Loading