-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
SQL handlers for custom media types #2825
Conversation
4243c4a
to
ee3d182
Compare
ee3d182
to
7ec4ee7
Compare
There's a problem in solving #2699 with the current aggregate interface. So having a replacement for our geojson is simple: create domain "application/geo+json" as jsonb;
create or replace function test.geoson_trans (state jsonb, next anyelement)
returns jsonb as $$
select state || extensions.ST_AsGeoJSON(next)::jsonb;
$$ language sql;
create or replace function test.geojson_final (data jsonb)
returns "application/geo+json" as $$
select jsonb_build_object('type', 'FeatureCollection', 'features', data);
$$ language sql;
create or replace aggregate test.geojson_agg (anyelement) (
initcond = '[]'
, stype = jsonb
, sfunc = geoson_trans
, finalfunc = geojson_final
);
curl "localhost:3000/shops" -H "Accept: application/geo+json"
{"type": "FeatureCollection", "features": [{"type": "Feature", "geometry": {"type": "Point", "coordinates": [-71.10044, 42.373695]}, "properties": {"id": 1, "address": "1369 Cambridge St"}}, {
"type": "Feature", "geometry": {"type": "Point", "coordinates": [-71.10543, 42.366432]}, "properties": {"id": 2, "address": "757 Massachusetts Ave"}}, {"type": "Feature", "geometry": {"type":
"Point", "coordinates": [-71.081924, 42.36437]}, "properties": {"id": 3, "address": "605 W Kendall St"}}]} If we want to add a create or replace function test.geojson_final (data jsonb)
returns "application/geo+json" as $$
select jsonb_build_object(
'type', 'FeatureCollection',
'crs', json_build_object(
'type', 'name',
'properties', json_build_object(
'name', 'EPSG:4326'
)
),
'features', data
);
$$ language sql; That's not a generic way though, ideally we use Find_SRID as recommend on #2699. For this, we could use FINALFUNC_EXTRA: create or replace function test.geojson_final (data jsonb, typ anyelement)
returns "application/geo+json" as $$
-- here, assume we enhance pg_typeof with what's required for find_SRID to work, i.e. getting the schema, name and column. Which is possible.
select jsonb_build_object('type', 'FeatureCollection', 'type', pg_typeof(typ), 'features', data);
$$ language sql;
create or replace aggregate test.geojson_agg (anyelement) (
initcond = '[]'
, stype = jsonb
, sfunc = geoson_trans
, finalfunc = geojson_final
, finalfunc_extra = true
);
curl "localhost:3000/shops" -H "Accept: application/geo+json"
{"type": "FeatureCollection", "type": "record", "features": [{"type": "Feature", "geometry": {"type": "Point", "coordinates": [-71.10044, 42.373695]}, "properties": {"id": 1, "address": "1369
Cambridge St"}}, {"type": "Feature", "geometry": {"type": "Point", "coordinates": [-71.10543, 42.366432]}, "properties": {"id": 2, "address": "757 Massachusetts Ave"}}, {"type": "Feature", "g
eometry": {"type": "Point", "coordinates": [-71.081924, 42.36437]}, "properties": {"id": 3, "address": "605 W Kendall St"}}]} Notice there's a customAggF :: QualifiedIdentifier -> SqlFragment
customAggF qi = fromQi qi <> "(_postgrest_t)" If we casted The casting won't work if resource embedding is done.
So maybe we can cast it only when embedding is not used? That would make the aggregate function unreliable though. Or maybe we should disallow resource embedding on non I believe this is something it should be solved in PostGIS itself. Maybe it's not worth it. The user can define different types of geojson aggs manually. |
I was under the wrong impression that we could do everything with aggregates. For instance, creating an aggregate that also strips nulls #1601. For this we would only need to apply a function to the final function of the aggregate.
create or replace function test.appjson_agg_final (data json)
returns "application/json" as $$
select json_strip_nulls(pg_catalog.json_agg_finalfn($1));
$$ language sql;
-- ERROR: function pg_catalog.json_agg_finalfn(json) does not exist
-- LINE 3: select json_strip_nulls(pg_catalog.json_agg_finalfn($1));
-- HINT: No function matches the given name and argument types. You might need to add explicit type casts.
create or replace aggregate test.appjson_agg (anyelement) (
initcond = '[]'
, stype = json
, sfunc = pg_catalog.json_agg_transfn
, finalfunc = appjson_agg_final
); The above won't work bc postgres=# \df+ json_agg_finalfn
List of functions
-[ RECORD 1 ]-------+------------------------------
Schema | pg_catalog
Name | json_agg_finalfn
Result data type | json
Argument data types | internal Wolfgang idea with CASTs would solve this case #1582 (comment). So looks we'll need CASTs anyway. CREATE CAST ("application/json" AS json) WITH FUNCTION json_strip_nulls; In general, CASTs allows us to have function composition. I'm thinking we should merge #2523 before going further on this feature. |
7ec4ee7
to
5505f64
Compare
Alternatively, I think this could be solved later with the |
I've just found out that thanks the new GRANT SET ON PARAMETER on pg 15, we can implement the original proposal with the custom GUC #1582. It works like this: create role manager login password 'secret';
grant all on schema public to manager;
set pgrst.bar to ''; -- this must be done otherwise the next statement fails with permission denied. I believe what happens is that the GUC is now registered as a placeholder.
grant set on parameter pgrst.bar to manager;
GRANT
begin;
set local role to manager;
create or replace function a_bar() returns text as $$
select 'hello';
$$ language sql
set pgrst.bar='foo';
CREATE FUNCTION This even works on RDS. The original proposal had the advantage of being able to define a default handler for The only thing that worries me is that the above could be considered a bug somehow and then it could be patched but it seems unlikely since it kinda makes sense (check the comment above).
|
While this solves your concern about the availability of this feature on cloud providers, it does not solve the inlining problem mentioned in #1582 (comment). The idea of using |
5505f64
to
71830ec
Compare
71830ec
to
45c05c5
Compare
6462f64
to
8ac8067
Compare
As a consequence of #2894, Maybe it's not so bad to make vendored media types non-overridable. After all users cannot override We could choose to drop |
8ac8067
to
61da62b
Compare
61da62b
to
7ce199c
Compare
it "will match image/png according to q values" $ do | ||
r1 <- request methodGet "/rpc/ret_image" (acceptHdrs "image/png, */*") "" | ||
liftIO $ do | ||
simpleBody r1 `shouldBe` readFixtureFile "A.png" | ||
simpleHeaders r1 `shouldContain` [("Content-Type", "image/png")] | ||
|
||
r2 <- request methodGet "/rpc/ret_image" (acceptHdrs "text/html,application/xhtml+xml,application/xml;q=0.9,image/png,*/*;q=0.8") "" | ||
liftIO $ do | ||
simpleBody r2 `shouldBe` readFixtureFile "A.png" | ||
simpleHeaders r2 `shouldContain` [("Content-Type", "image/png")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clears the case in #1462, replace image/png
for image/webp
. This is more about correct content negotiation.
-- https://github.com/PostgREST/postgrest/issues/1371#issuecomment-519248984 | ||
it "will match a custom text/csv with BOM" $ do | ||
r <- request methodGet "/lines" (acceptHdrs "text/csv") "" | ||
liftIO $ do | ||
simpleBody r `shouldBe` readFixtureFile "lines.csv" | ||
simpleHeaders r `shouldContain` [("Content-Type", "text/csv; charset=utf-8")] | ||
simpleHeaders r `shouldContain` [("Content-Disposition", "attachment; filename=\"lines.csv\"")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clears #1371 (comment), includes a BOM CSV plus attachment header. Had to read from a file otherwise couldn't match the BOM on Haskell
7ce199c
to
707b100
Compare
BREAKING CHANGE Can be done later with custom media types
BREAKING CHANGE Can be done later with custom media types
Accept
707b100
to
ee842eb
Compare
ee842eb
to
cb87772
Compare
* test text/html and drop HtmlRawOutputSpec.hs * all tests passing, removed all pendingWith * make functions compatible with pg <= 12 * move custom media types tests to own spec * anyelement aggregate * apply aggregates without a final function * overriding works * overriding anyelement with particular agg * cannot override vendored media types * plan spec works with custom aggregate * renamed media types to make clear which ones are overridable * correct content negotiation with same weight * text/tab-separated-values media type * text/csv with BOM plus content-disposition header
cb87772
to
33c008b
Compare
Ok this should be ready to go. It implements the first stage idea discussed on #1582 (comment). This means that the CASTs idea can be built on top. The code supports the idea of "handlers", not only aggregates, meaning a regular function can be the final one applied. handlerF :: Maybe Routine -> QualifiedIdentifier -> MediaHandler -> SQL.Snippet
handlerF rout target = \case
BuiltinAggArrayJsonStrip -> asJsonF rout True
BuiltinAggSingleJson strip -> asJsonSingleF rout strip
-- these ones are overridable
BuiltinOvAggJson -> asJsonF rout False
BuiltinOvAggGeoJson -> asGeoJsonF
BuiltinOvAggCsv -> asCsvF
CustomFunc funcQi -> customFuncF rout funcQi target
NoAgg -> "''::text"
customFuncF :: Maybe Routine -> QualifiedIdentifier -> QualifiedIdentifier -> SQL.Snippet
customFuncF rout funcQi target
| (funcReturnsScalar <$> rout) == Just True = fromQi funcQi <> "(_postgrest_t.pgrst_scalar)"
| otherwise = fromQi funcQi <> "(_postgrest_t::" <> fromQi target <> ")"
There might be some rough edges, but it already solves a lot of issues. Merging it later will cost a lot (it already took a lot of rebases), so I'd rather merge it at this stage and improve it later. |
Merging. Will continue with #2826 to solve the remaining issues. |
With |
This was removed in #2825.
Summary
Custom media types are now possible on RPC and tables by using scalar functions and aggregates respectively.
Docs in progress: PostgREST/postgrest-docs#689
Examples
text/csv
handler that includes a BOM (byte order mark) plusContent-Disposition
header:application/geo+json
generic handler (anyelement
can work thanks toST_AsGeoJSON
being generic):Breaks
application/octet-stream
,text/plain
,text/xml
builtin support for scalar resultsapplication/openapi+json
media type for db-root-specCloses
Accept: text/plain, application/json
#2170Pending
Content-Type
#2826