Skip to content
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

Get the EXPLAIN output of a request #2354

Closed
steve-chavez opened this issue Jun 28, 2022 · 9 comments · Fixed by #2368
Closed

Get the EXPLAIN output of a request #2354

steve-chavez opened this issue Jun 28, 2022 · 9 comments · Fixed by #2368
Labels
enhancement a feature, ready for implementation

Comments

@steve-chavez
Copy link
Member

steve-chavez commented Jun 28, 2022

Sometimes it's difficult to understand why a simple request like /tbl?id=eq.1 can be slow while it's equivalent in SQL is fast. This is usually due to a bad RLS policy that only applies to a postgREST role.

We could support an Accept: application/vnd.pgrst.explain+json media type that gets the EXPLAIN (FORMAT JSON) output into a response. That would clarify that the query is expensive and is not because of us doing some extra processing.

An EXPLAIN ANALYZE could also be supported with Accept: application/vnd.pgrst.explain-analyze+json, coupled with Prefer: tx=rollback it would be safe to do. We might also consider applying tx=rollback by default for it.


References:

@steve-chavez steve-chavez added the idea Needs of discussion to become an enhancement, not ready for implementation label Jun 28, 2022
@steve-chavez
Copy link
Member Author

Actually EXPLAIN (FORMAT JSON) ANALYZE is not allowed in pg(gives syntax error). So we only need a single media type.

Searching IANA types, I found some related to cost(alto-endpointcost+json, alto-costmap+json, etc) but those doesn't map to our use case.

So we should go with our own vendor media type: application/vnd.pgrst.plan. Taking advantage of the different formats in EXPLAIN, we can do application/vnd.pgrst.plan+{text,xml,json,yaml}.

@steve-chavez
Copy link
Member Author

Previous discussion of using the http TRACE method for debugging

Mentioned before, but repeating it here:

A client MUST NOT send a message body in a TRACE request.

So unfortunately that leaves TRACE out. Still, because of the different formats supported, the media type seems like the right interface.

@steve-chavez
Copy link
Member Author

Getting the pg 14 Query ID would be good for debugging too. That way a request can be easily mapped to a query in the logs.

Seems it's possible to get it with EXPLAIN VERBOSE as mentioned in the blog post but I wonder if we could get it for every query somehow, then we could expose it in a header(Query-Identifier: 1382294904069104546).

@wolfgangwalther
Copy link
Member

Actually EXPLAIN (FORMAT JSON) ANALYZE is not allowed in pg(gives syntax error). So we only need a single media type.

It's EXPLAIN (ANALYZE, FORMAT JSON).

@steve-chavez
Copy link
Member Author

Oh, thanks for clarifying that Wolfgang.

I guess we should have two media types then. Hm, I think it'd be cleaner to just have one. Maybe we can just do the EXPLAIN ANALYZE with the ROLLBACK.

@steve-chavez
Copy link
Member Author

steve-chavez commented Jul 3, 2022

There's a problem with using a media type for this. We generate different queries for each of our supported media types. If we use application/vnd.pgrst.plan+{text,xml,json,yaml} for the different outputs of the EXPLAIN, how would we control the generated query..

I guess we could match the application/vnd.pgrst.plan+json to the generated json_agg query plus the EXPLAIN FORMAT JSON and application/vnd.pgrst.plan+text to the generated string_agg plus EXPLAIN FORMAT TEXT. In that case a new media type doesn't help that much, so I think a Prefer header would be more suitable.

We could reuse our return for this:

  • EXPLAIN: Prefer: return=plan
  • EXPLAIN ANALYZE: Prefer: return=analysis

Or maybe we could join all three into one return plus parameters:

# EXPLAIN
Prefer: return=plan
# EXPLAIN ANALYZE
Prefer: return=plan; analyze
# return the generated sql
Prefer: return=plan; statement

application/sql would have the same problem. In this case maybe a return=sql(or return=generated) would do.

Bad thing about this is that application/sql would give us a clean output of the generated SQL and with application/json we'd have to stuff the query inside a json string.


Another option I checked is having multiple suffixes on a media type(possible) but that wouldn't help it seems.(it's about the subtypes that a response complies)

An alternative would be having multiple plan media types, like:

## would generate the json_agg query
application/vnd.pgrst.plan-json+{text,xml,json,yaml}
## would generate the string_agg query
application/vnd.pgrst.plan-text+{text,xml,json,yaml}

Which is weird, but it allows for controlling the explain format plus the generated query.

It would also allow us to provide a clean SQL output of the generated query by doing:

application/vnd.pgrst.plan-json+sql
application/vnd.pgrst.plan-text+sql

@steve-chavez
Copy link
Member Author

Since it leaks internal details of the table, we can enable the above preference with a config. In the same way is done for the Prefer: tx=rollback header with db-tx-end. It could be db-allow-plan or db-allow-explain.

@steve-chavez
Copy link
Member Author

steve-chavez commented Jul 4, 2022

Settling on a media type with parameters plus suffixes for this. This has the advantage of being able to generate the different queries plus obtain the EXPLAIN in different formats.

# By default get the Accept: application/json generated query plus EXPLAIN FORMAT JSON
application/vnd.pgrst.plan

# Get the Accept: application/json generated query explicitly plus EXPLAIN FORMAT TEXT
application/vnd.pgrst.plan+text;for="application/json" 

# Get the Accept: text/plain generated query explicitly plus EXPLAIN output in XML
application/vnd.pgrst.plan+xml;for="text/plain" 

Since the for parameter has the media type, it could later be extended to support custom Accept types too(#1582).

Edit: According to the token rules, we cannot use "/" inside a parameter unless it's within double quotes.

Delimiters are chosen from the set of US-ASCII visual characters not allowed in a token (DQUOTE and "(),/:;<=>?@[]{}")
quoted-string = DQUOTE *( qdtext / quoted-pair ) DQUOTE
qdtext = HTAB / SP /%x21 / %x23-5B / %x5D-7E / obs-text

(x2F(/) is included in the x23-5B range)

With media type parameters, all the options in EXPLAIN can be manipulated as well:

application/vnd.pgrst.plan;analyze;verbose;costs;timing

Edit: Just noted that it's not allowed to have parameters without a value. I've seen other types that use commas inside double quotes for multiple values, e.g. codecs:

video/webm;codecs="vp8,vorbis"

However this might cause an issue with the parseHttpAccept function we use, since it splits by commas.

According to the token rules, we could use | though:

application/vnd.pgrst.plan;options=analyze|verbose|costs|timing

For getting the generated SQL, we could do:

# Get the Accept: text/xml generated query SQL
application/vnd.pgrst.plan+sql;for=text/xml 

Though it doesn't really make sense to have the same plan media type(with the same parameters) for getting SQL. So we could do:

# by default get the generated sql(plain) for Accept: application/json 
application/sql 

# get the generated sql for text/xml
application/vnd.pgrst.sql;for="text/xml"

# in theory we could output the sql wrapped in json like so(but maybe not worth it)
application/vnd.pgrst.sql+json;for="text/xml"

@steve-chavez
Copy link
Member Author

steve-chavez commented Jul 5, 2022

This library already has more complete(subtypes, parameters) parsing of media types than the functions we have in core: https://hackage.haskell.org/package/http-media-0.8.0.0/docs/Network-HTTP-Media-MediaType.html#g:2

Edit: doesn't have suffix support zmthy/http-media#15

@steve-chavez steve-chavez added enhancement a feature, ready for implementation and removed idea Needs of discussion to become an enhancement, not ready for implementation labels Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement a feature, ready for implementation
Development

Successfully merging a pull request may close this issue.

3 participants