add: accept application/vnd.pgrst.sql requests by wolfgangwalther · Pull Request #5054 · PostgREST/postgrest

wolfgangwalther · 2026-06-27T20:02:52Z

This allows returning the request's SQL query by setting the Accept: application/vnd.pgrst.sql header on the client-side. I opted to use a mimetype in our vendor namespace, because this needs to support a for= parameter from the beginning - queries differ depending on which mimetype would normally be requested.

I mostly copied my way along the implementation of the application/vnd.pgrst.plan header and made adjustments where necessary.

This still has a few rough edges. Test coverage is not very meaningful, yet - ideally we'd do some snapshot/golden tests for these basic queries, but I couldn't get hspec-golden to work, yet. Also, some parts of the code theoretically support this, for example, on OpenAPI requests as well, but others don't, so that's a bit inconsistent.

Compared to #4012, this does not attempt to return parameter values. It's highly valuable without, I think.

For now, this should allow me to run the loadtests with it to see whether that gives us more stable results.

This allows returning the request's SQL query by setting the `Accept: application/vnd.pgrst.sql` header on the client-side. I opted to use a mimetype in our vendor namespace, because this needs to support a `for=` parameter from the beginning - queries differ depending on which mimetype would *normally* be requested. I mostly copied my way along the implementation of the `application/vnd.pgrst.plan` header and made adjustments where necessary. Resolves PostgREST#3580

wolfgangwalther · 2026-06-27T20:09:27Z

+-- TODO: maybe patch upstream hasql-dynamic-statements so we have a less hackish way to convert
+-- the SQL.Snippet or maybe don't use hasql-dynamic-statements and resort to plain strings for the queries and use regular hasql
+renderSnippet :: SQL.Snippet -> ByteString
+renderSnippet snippet =
+  let SQL.Statement sql _ _ _ = SQL.dynamicallyParameterized snippet decoder False
+      decoder = HD.noResult -- unused
+  in
+    sql
+


Note: This function is copied entirely from Logger.hs, since it is meant to be temporary anyway.

We could move it to our vendored hasql, once we get there.

@wolfgangwalther Could vendoring hasql-dynamic-statements be a good first step towards #4823? It seems it'd be simplest step and there's a need here.

Could vendoring hasql-dynamic-statements be a good first step towards #4823?

Done in #5084.

wolfgangwalther · 2026-06-27T20:09:51Z

+      -- TODO the plan should have its own MediaHandler instead of relying on MediaType
+      m@(MTVndSQL mType)                          -> mtPlanToNothing $ ((,) . fst <$> lookupHandler mType) <*> pure m


TODO copied 1:1 from above.

wolfgangwalther · 2026-06-27T20:12:34Z

-      MTVndPlan{} -> DbCrud True pl
-      _           -> DbCrud False pl
+      MTVndPlan{}          -> DbCrud True pl
+      MTVndSQL MTVndPlan{} -> SQLOnly $ DbCrud True pl


Note: Should probably add a test-case for the "return the SQL of an EXPLAIN" scenario.

That doesn't seem that useful, which makes me think perhaps we should reuse the vnd.pgrst.plan type?

It could be vnd.pgrst.plan+sql?

I thought about that, but the first challenge will be to match it on the MTVndPlan type, which supports all kinds of options and such - none of which make any sense for the sql case. Essentially this shows that this is just a different thing in reality as well and should not be modeled as the same type.

Returning the SQL or running an EXPLAIN are two very different things, imho. I'd go as far as saying: We should not even tie application/sql to db-plan-enabled at all. We should just allow it unconditionally. I mean - what's the downside? Our security model is supposed to be very similar to "we expect the user to be able to access the SQL database directly anyway, we're just providing the HTTP API for it", so I don't see a risk in letting any user know which SQL query we'd run.

I mean - what's the downside?

I don't think all users would be fine with that. Recently we had some that didn't want the hints #4667 and this feature does give much more information about implementation details.

We should not even tie application/sql to db-plan-enabled at all

I do agree in that they're different, but I don't see any other option than adding another config 😞 .

Yeah, I assumed you were going to say that. That's why I tied it to db-plan-enabled, like you suggested earlier - it doesn't hurt either. There is very little use-case for "I want to enable one of the two, but not the other" anyway.

I only mentioned it to make it clear that I think this feature is distinct enough from explain/plan, that we should not do more than putting it behind the same config option. We should treat both media types separately otherwise.

wolfgangwalther · 2026-06-27T20:13:08Z

+actionResult MainQuery{..} (SQLOnly (DbCrud _ plan)) _ _ _ =
+  pure $ SQLResult (pMedia plan) $ BS.intercalate "\n\n" $ filter (/= mempty) snipts
+  where
+    snipts = renderSnippet <$> [mqTxVars, fromMaybe mempty mqPreReq, mqMain, fromMaybe mempty mqExplain]


This needs to add support for OpenAPI requests.

Needs a test-case and some more pattern patching elsewhere as well.

I'd be fine if openapi rejects this media type.

Even with postgrest-openapi I'm not sure if we'd always hit the db.

I'd be fine if openapi rejects this media type.

I'll actually need the openapi endpoint to respond with SQL, otherwise #5055 is almost pointless - the openapi requests just dominate the loadtest right now, they would do so even more when all others don't hit the DB anymore. But once the openapi requests don't hit the DB either, the mixed loadtests are actually much more balanced between the requests. This might, after all, be the biggest effect that PR will have.

wolfgangwalther · 2026-06-27T20:15:57Z

+      liftIO $ do
+        resHeaders `shouldSatisfy` elem ("Content-Type", "application/vnd.pgrst.sql; for=\"application/json\"; charset=utf-8")
+        resHeaders `shouldSatisfy` notZeroContentLength
+        resStatus `shouldBe` Status { statusCode = 200, statusMessage="OK" }
+


If not snapshot-testing, these tests should at least test key components of the query string in the body.

steve-chavez · 2026-06-27T21:16:31Z

+SQL query
+---------
+
+You can get the raw SQL query of a request by adding the ``Accept: application/sql`` header.


This header is outdated?

Also I get the need of a vendor mimetype but perhaps we can add application/sql for the default case? (Should be the json handler)

Not adamant on the above, just an idea if implementation is simple enough.

wolfgangwalther force-pushed the accept-sql branch from 3f56247 to ec2a0ae Compare June 27, 2026 20:04

wolfgangwalther linked an issue Jun 27, 2026 that may be closed by this pull request

Returning generated SQL #3580

Open

wolfgangwalther force-pushed the accept-sql branch from ec2a0ae to 1030707 Compare June 27, 2026 20:07

wolfgangwalther added 2 commits June 27, 2026 22:11

chore: fix spelling in code comment

94c9fa9

wolfgangwalther commented Jun 27, 2026

View reviewed changes

wolfgangwalther force-pushed the accept-sql branch from 1030707 to d57af98 Compare June 27, 2026 20:16

wolfgangwalther mentioned this pull request Jun 27, 2026

nix(loadtest): avoid hitting the database when possible #5055

Draft

steve-chavez reviewed Jun 27, 2026

View reviewed changes

		-- TODO the plan should have its own MediaHandler instead of relying on MediaType
		m@(MTVndSQL mType) -> mtPlanToNothing $ ((,) . fst <$> lookupHandler mType) <*> pure m

Uh oh!

Uh oh!

Conversation

wolfgangwalther commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

wolfgangwalther commented Jun 27, 2026 •

edited

Loading