Extended computed columns

steve-chavez commented 1 year ago

Problem

It's not possible to apply parameters to computed columns. If I'd want to use the pgroonga_highlight_html function like:

SELECT pgroonga_highlight_html(content, ['fast', 'PostgreSQL']) FROM samples;

It's currently not possible to create a computed and apply the ['fast', 'PostgreSQL'] argument in:

GET /samples?select=*,wrapped_pgroonga_highlight_html

Proposal

Taking Wolfgang's syntax idea for aggregates(ref) and the SEARCH http method with underscore operators(ref), we could support:

SEARCH /samples?select=*,content.wrapped_pgroonga_highlight_html(body->keywords)&id=eq.1

{
  "keywords": ["fast", "PostgreSQL"]
}

With the extended computed column:

create function wrapped_pgroonga_highlight_html(tbl samples, keywords text[]) as $$
  SELECT pgroonga_highlight_html(tbl.content, keywords); 
$$ language sql stable;

Drawbacks

Adding computed columns for each output expression in select has an impact on db state and could be considered a sort of "damage"(like mocks on unit tests, see ref). To avoid this we'd need to have a way to allowlist functions to call them directly on select.

I think computed columns is the only safe way to expose transformation on select though. One can also define generic computed columns(taking an anyelement param) to avoid adding too much of these.

adrinr commented 1 year ago

I am actually tackling a similar issue, this would be ideal for us as well!

steve-chavez commented 1 year ago

Taking the idea from https://github.com/PostgREST/postgrest/issues/915#issuecomment-1376539009, a refinement to the above syntax would be:

SEARCH /samples?select=*,$f.wrapped_pgroonga_highlight_html(content,$body.keywords)&id=eq.1

{
  "keywords": ["fast", "PostgreSQL"]
}

steve-chavez commented 1 year ago

To avoid a more complex syntax for now, we could allow:

GET /samples?select=*,wrapped_pgroonga_highlight_html&wrapped_pgroonga_highlight_html.keywords=["fast", "PostgreSQL"]

or

GET /samples?select=*,$f.wrapped_pgroonga_highlight_html&wrapped_pgroonga_highlight_html.keywords=["fast", "PostgreSQL"]

wolfgangwalther commented 1 year ago

To avoid a more complex syntax for now, we could allow:
GET /samples?select=*,wrapped_pgroonga_highlight_html&wrapped_pgroonga_highlight_html.keywords=["fast", "PostgreSQL"]

Uh, this is interesting. I like the symmetry between passing arguments to RPCs and other functions: Whenever another query string parameter does not have an operator, it's an argument. This can be a top-level argument for the RPC, or a nested argument with function names.

steve-chavez commented 1 year ago

Yeah, I like that one too. Also with aliases it can be shorter:

GET /samples?select=*,highlight:wrapped_pgroonga_highlight_html&highlight.keywords=["fast", "PostgreSQL"]

This should be straightforward to support because it reuses existing parsers.

steve-chavez commented 1 year ago

Whenever another query string parameter does not have an operator, it's an argument. This can be a top-level argument for the RPC, or a nested argument with function names.

I've just noticed that might conflict with the OpenAPI fix on https://github.com/PostgREST/postgrest/issues/1970#issuecomment-1382613004.

So I think we should change the syntax to:

GET /samples?select=*,highlight:wrapped_pgroonga_highlight_html&highlight.keywords=arg.["fast", "PostgreSQL"]

(Note the highlight.keywords=arg.)

I'm sure that will make the transition to operators to the left side easier https://github.com/PostgREST/postgrest/issues/2066

PostgREST / postgrest