Skip to content

Conversation

cptwunderlich
Copy link

Updating postgrest to fix broken plaintext and json routes.

@cptwunderlich
Copy link
Author

I also looked at the "updates" benchmark and it should work in theory. However, your benchmark calls that endpoint as a GET, even though it modifies the resource.
PostgREST is creating a REST API based on your DB schema. And for GET requests, it sets a READ ONLY transaction. So updates fail. Using e.g. POST would work.

@cptwunderlich cptwunderlich marked this pull request as draft August 21, 2025 16:09
@cptwunderlich
Copy link
Author

Oops, I realized the fortunes benchmark is also broken. Will add that tomorrow.

@cptwunderlich cptwunderlich marked this pull request as ready for review August 22, 2025 09:29
@cptwunderlich
Copy link
Author

Locally, I get a failure for "fortunes":

Only 506 rows read in the database out of roughly 6144 expected.

I'm not sure what's happening there though.

END
$$ language plpgsql volatile;
create or replace function fortunes() returns "text/html" as $$
select set_config('response.headers', '[{"Content-Type": "text/html; charset=utf-8"}]', true);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @cptwunderlich,

This line shouldn't be necessary. PostgREST will automatically set the content type to the "text/html" domain.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this adds the type "text/html", but the benchmark harness expects an explicit value of "text/html; charset=UTF-8".
If you remove it, the test fails with:

   FAIL for http://tfb-server:3000/rpc/fortunes
     Invalid Content-Type header, found "text/html", did not match "^text/html; ?charset=(UTF|utf)-8$".

I can add a comment to clarify.

There are some oddities in this benchmark. E.g., the "updates" benchmark will never work for postgREST, bc. they use a "GET" request for that and you just get a read-only transaction for that in postgrest....

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this adds the type "text/html", but the benchmark harness expects an explicit value of "text/html; charset=UTF-8".

Got it, thanks for clarifying.

the "updates" benchmark will never work for postgREST, bc. they use a "GET" request for that and you just get a read-only transaction for that in postgrest....

Yeah, that is weird on how they landed on GET for this. There are no escape hatches on PostgREST for doing a write in a GET.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the update benchmark should just be omitted

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, it's not in the config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants