Skip to content

Returning generated SQL #3580

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

Open
gc-ft opened this issue Jun 11, 2024 · 12 comments · May be fixed by #4012
Open

Returning generated SQL #3580

gc-ft opened this issue Jun 11, 2024 · 12 comments · May be fixed by #4012
Labels
difficulty: beginner Pure Haskell task

Comments

@gc-ft
Copy link

gc-ft commented Jun 11, 2024

In issue #2354 it is discussed multiple times that displaying the generated SQL might be interesting too. It seems like this feature was in the end not implemented.

I unterstand that there are other ways to retrieve the SQL query (for example through logs and using EXPLAIN VERBOSE to get the query ID to make sure to find it easily...

However, any explain plan returned through the plan accept header implemented is always going to be easier to understand, judge and also act upon with the SQL query present in the same data without having to implement extra steps which enable logging queries. It seems a lot more efficient to have the option to just return the SQL query right before the plan in the same reply?

I understand security considerations need to be taken, but this could be implemented via an extra config as db-allow-generated-sql or db-allow-plan-sql if it is only usable in combination with plan.

@wolfgangwalther wolfgangwalther added the idea Needs of discussion to become an enhancement, not ready for implementation label Jun 11, 2024
@ggam
Copy link

ggam commented Jun 11, 2024

Big +1 on this. It would be very useful for debugging purposes.

@YonatanHanan
Copy link

would really help out

@steve-chavez
Copy link
Member

Now that we output part of the generated SQL on the logs:

We can implement this.

The idea would be to output the generated SQL when Accept: application/sql is requested. Later, it might be possible to do Accept: application/vnd.pgrst.plan+sql to output the SQL statement with the EXPLAIN output.

This only when https://docs.postgrest.org/en/latest/references/configuration.html#db-plan-enabled is true.

@steve-chavez steve-chavez added breaking change A bug fix or enhancement that would cause a breaking change difficulty: beginner Pure Haskell task and removed idea Needs of discussion to become an enhancement, not ready for implementation breaking change A bug fix or enhancement that would cause a breaking change labels Mar 19, 2025
@zachblume
Copy link

zachblume commented Mar 19, 2025

Portaling a comment from another issue about why this would be a great solution for my use case:
#3942 (comment)

@taimoorzaeem taimoorzaeem self-assigned this Apr 7, 2025
@taimoorzaeem
Copy link
Collaborator

taimoorzaeem commented Apr 7, 2025

@steve-chavez How should we implement only the application/sql part for now?

  • What HTTP methods in the request should be allowed for this?
  • Any new configs?
  • Do we return the full sql or just the main-query?

I think we might need a refactor for this but not sure.

@laurenceisla
Copy link
Member

  • What HTTP methods in the request should be allowed for this?

Every method that executes SQL. For example, OPTIONS wouldn't return anything since it doesn't trigger a SQL query.

  • Any new configs?

I think so, yes. Maybe, something like db-query-enabled? Otherwise we could reuse the db-plan-enabled (not sure about this).

  • Do we return the full sql or just the main-query?

For now, since just the log-query = main-query is implemented, then we can start only with the main-query here, too. It would be the default for Accept: application/sql, later more parameters would be used for the other SQL queries.

My opinions, Steve can correct/add more to these.

@fauh45
Copy link

fauh45 commented Apr 7, 2025

Hi all, sorry to suddenly comes in. @zachblume has notify me of this, and I'm currently actively working on getting the Accept: application/sql working on a fork.

End goal for me would be to get the main, count, and also template values on the query as well. Just want to put it out here as well!

@taimoorzaeem taimoorzaeem removed their assignment Apr 8, 2025
@steve-chavez
Copy link
Member

Otherwise we could reuse the db-plan-enabled (not sure about this).

The db-plan-enabled config should be enough (as mentioned previously). The SQL query is intrinsically linked to the plan, so it's not necessary to add a new config.

End goal for me would be to get the main, count, and also template values on the query as well.

By template values you mean the placeholder values ($1, $2, $3) of the parametrized query ? To do that I think we need #3934.

@zachblume
Copy link

zachblume commented Apr 8, 2025

By template values you mean the placeholder values ($1, $2, $3) of the parametrized query ? To do that I think we need #3934.

Yes that's what @fauh45 was referring to, since we need them for our use-case of mutating queries a bit post-generation and then actually running them from application.

Just read #3934 and couldnt immediately figure out the connection to template values. Would you clarify? I think we'll want to implement this narrowly but can expand to cover as much of #3934 as is needed for this

@fauh45
Copy link

fauh45 commented Apr 10, 2025

Hi all, got the first WIP working where you can query using Accept: application/sql to return the main query (currently only on the read, and write). You can see all the changes on my for for PostgREST (https://github.com/fauh45/postgrest/commits/main/ on main branch).

Read

[nix-shell:~/Code/postgrest]$ curl -H "Accept: application/sql" http://localhost:3000/call_sessions -vv
* Host localhost:3000 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:3000...
* connect to ::1 port 3000 from ::1 port 60370 failed: Connection refused
*   Trying 127.0.0.1:3000...
* Connected to localhost (127.0.0.1) port 3000
> GET /call_sessions HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/8.7.1
> Accept: application/sql
>
* Request completely sent off
< HTTP/1.1 200 OK
< Transfer-Encoding: chunked
< Date: Thu, 10 Apr 2025 04:47:45 GMT
< Server: postgrest/12.3 (pre-release)
< Content-Type: application/sql; charset=utf-8
<
* Connection #0 to host localhost left intact
WITH pgrst_source AS ( SELECT "public"."call_sessions".* FROM "public"."call_sessions"  )  SELECT null::bigint AS total_result_set, pg_catalog.count(_postgrest_t) AS page_total, ''::text AS body, nullif(current_setting('response.headers', true), '') AS response_headers, nullif(current_setting('response.status', true), '') AS response_status, '' AS response_inserted FROM ( SELECT * FROM pgrst_source ) _postgrest_t

Mutate

[nix-shell:~/Code/postgrest]$ curl -H "Accept: application/sql" -H "Content-Type: application/json" -X POST http://localhost:3000/test -d '{"col": "data"}' -vvv
Note: Unnecessary use of -X or --request, POST is already inferred.
* Host localhost:3000 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:3000...
* connect to ::1 port 3000 from ::1 port 62312 failed: Connection refused
*   Trying 127.0.0.1:3000...
* Connected to localhost (127.0.0.1) port 3000
> POST /test HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/8.7.1
> Accept: application/sql
> Content-Type: application/json
> Content-Length: 15
>
* upload completely sent off: 15 bytes
< HTTP/1.1 200 OK
< Transfer-Encoding: chunked
< Date: Thu, 10 Apr 2025 05:27:04 GMT
< Server: postgrest/12.3 (pre-release)
< Content-Type: application/sql; charset=utf-8
<
* Connection #0 to host localhost left intact
WITH pgrst_source AS (INSERT INTO "public"."test"("col") SELECT "pgrst_body"."col" FROM (SELECT $1 AS json_data) pgrst_payload, LATERAL (SELECT "col" FROM json_to_record(pgrst_payload.json_data) AS _("col" text) ) pgrst_body  RETURNING 1) SELECT '' AS total_result_set, pg_catalog.count(_postgrest_t) AS page_total, array[]::text[] AS header, ''::text AS body, nullif(current_setting('response.headers', true), '') AS response_headers, nullif(current_setting('response.status', true), '') AS response_status, '' AS response_inserted FROM (SELECT * FROM pgrst_source) _postgrest_t

I had to add custom Media Type MTApplicationSQL for application/sql. As well as adding a new item on union QueryResult, so that I can do custom handling on actionResponse.

Also can you guys validate my understanding that all the mutate query (or queries that have body) the raw value of the body just got passed to postgres and parsed on Postgres itself? Currently what I've done to get the parametrized value is to just return updBody or insBody, not sure if that assumption is always correct though. Here's my WIP fauh45@88556f7

I know my code might not be up to standard (last time I barely use Haskell was in University 3 years ago), so if you guys have some time, may you guys be able to critic my code? Any input would be amazing!

@wolfgangwalther
Copy link
Member

Also can you guys validate my understanding that all the mutate query (or queries that have body) the raw value of the body just got passed to postgres and parsed on Postgres itself? Currently what I've done to get the parametrized value is to just return updBody or insBody, not sure if that assumption is always correct though. Here's my WIP fauh45@88556f7

I know my code might not be up to standard (last time I barely use Haskell was in University 3 years ago), so if you guys have some time, may you guys be able to critic my code? Any input would be amazing!

To get feedback on your code, please open a PR. It doesn't need to be perfect right away, but giving feedback in the issue / on a branch without PR is harder than it needs to be.

Also: Make sure you write some kind of tests. Not only for us, but also for you. Your development will be much faster (and you can refactor with more confidence), if you have some tests running along.

@fauh45
Copy link

fauh45 commented Apr 10, 2025

Sounds good! I'll make a PR for the application/sql raw main SQL first for now. I'll be sure to create some test for it too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: beginner Pure Haskell task
Development

Successfully merging a pull request may close this issue.

9 participants