Comment directive conflicts with postgREST

[tonyxiao]

@see https://postgrest.org/en/stable/api.html#openapi-support

Not sure whether there is anything we can do at all other than updating the postgREST. However want to bring attention to it in case there is a workaround.

[olirice]

I think this may need to move over to postgrest

there's no work-around we could introduce in pg_graphql that would impact how postgrest displays comments

cc @steve-chavez
everything pg_graphql related is scoped to the @graphql(<json>) block

[steve-chavez]

On the postgREST side, we have received a similar issue from a postgraphile user before.

there's no work-around we could introduce in pg_graphql that would impact how postgrest displays comments

PostgREST displays the comment as it is, the problem is that both postgraphile and pg_graphql have an ad hoc format for metadata. We haven't used metadata on COMMENTs for PostgREST exactly for this reason(conflicts with other tools, besides not being invasive to existing databases).

IMO, what should change is the format for pg_graphql metadata. If it were to use a standard format, like YAML, for example like this:

COMMENT ON TABLE tbl $$
graphql:
  config: val
  comment: my graphql comment
$$;

Then postgREST could parse this YAML, ignore the graphql value and potentially reserve a top rest value for its own uses. Otherwise I don't see how we could support an exception in such an ad hoc way.

[olirice]

what should change is the format for pg_graphql metadata. If it were to use a standard format, like YAML, for example like this:

the difference here being that the @graphql() part is non-standard?

Could you give me an example of what a comment that has a GraphQL description and a PostgREST description would look like in this structure? (I'm not clear if PostgREST would also be adopting YAML or if it is only being used to detect and ignore pg_graphql configuration)

Would it be:

COMMENT ON TABLE tbl $$
graphql:
  description: my graphql description

postgrest:
  description: my openapi description
$$;

or

COMMENT ON TABLE tbl $$
graphql:
  description: my graphql description

my openapi description
$$;

[tonyxiao]

I would expect some top level fields that all tools can choose to use. If we want to document our data model for example it's gonna be quite annoying to have to repeat ourselves once for graphql and once for rest.

Maybe we can borrow from markdown and yaml frontmatter, where it is possible to both have structured and unstructured (markdown) data?

COMMENT ON TABLE tbl $$

graphql:
  description: my graphql description
rest:
  settings: my rest settings
---
# My awesome table

- [ ] Will rock your socks off

$$;

The top level description would be the default, and it would be the value of the markdown body. Then of course we can have tool specific overrides.

[tonyxiao]

Btw I’m currently thinking of we proxy the openAPI response from pgRest and modify the document to parse the @graphql directive ourselves and expose that to clients. Though this really feels like something that ought to work at a supabase level given that rest & graphql are both natively supported on the platform, feels weird that there isn’t a way to document API across both of them at the same time easily.

[steve-chavez]

Maybe we can borrow from markdown and yaml frontmatter, where it is possible to both have structured and unstructured (markdown) data?

That looks good, we had a similar proposal on postgREST before: PostgREST/postgrest#2028 (comment).

Reference: https://assemble.io/docs/YAML-front-matter.html

Though this really feels like something that ought to work at a supabase level given that rest & graphql are both natively supported on the platform

True. Both tools need to agree on a format for this.

[olirice]

@steve-chavez currently the unexpected output is only visible in PostgREST 's OpenAPI doc so I think it make sense to continue this conversation over there (on PostgREST/postgrest#2028 (comment) or a separate issue).

I'm sold on adopting a common format like yaml (or toml). It'd only take a couple days to implement backwards compatibly in pg_graphql so let me know when you're ready and we can knock out a spec and get it implemented in both