Fix parallel spec tests still mutating some sequences

[wolfgangwalther]

Noted after merging #1797, but unrelated to that PR, we still have some tests that depend on the order in which they are run. Some kind of rare race-condition when running the tests.

Error here:
https://app.circleci.com/pipelines/github/PostgREST/postgrest/850/workflows/1c7c98b4-8081-4239-96a8-fef6a9fb5f1e/jobs/8487

[simian-loco]

Quick question @wolfgangwalther which may be related to this issue (but perhaps not). Is the return from prefer: return-representation / other mutating requests guaranteed to return after COMMIT has occurred (and thus after all AFTER triggers)? What about if the after trigger is DEFERRABLE?

[wolfgangwalther]

In fact it's completely unrelated to this issue ;)

However I made a quick test as follows:

create table t (
  c TEXT
);

create function sleep_raise() returns trigger
language plpgsql as $$begin
  perform pg_sleep(5);
  raise 'failed';
end$$;

create constraint trigger def after insert on t
deferrable initially deferred for each row
execute function sleep_raise();

Now, when I run a POST request with return=representation to this table, the request will take 5 seconds and then return with an error - and not the inserted row. So the constraint trigger is indeed executed before any response is sent.

Note, however, that this is not the case, when using Prefer: tx=rollback. The transaction will be rolled back without the constraint triggers firing. I guess we need to fix that. I'll create an issue for it.

[simian-loco]

Thanks for the info! I've concluded that my HTTP-based, python multi-threaded tests are most likely just running too quickly, resulting in phantom read anomalies which are throwing errors.

Before each and every test run, I create multiple user_accounts, each within a BEFORE trigger GRANTing <role> to authenticator; but sometimes an immediate, subsequent request using that new <role>'s bearer token throws a 403 complaining that authenticator does not have permission to SET ROLE <role>, I am assuming this is because of a phantom read, the GRANT not having been processed yet. I could probably fix this in a couple of ways (e.g. isolation level to serializable or smart request throttling) but for now I am just commentating the fail as most likely being anomalous and if the dev is worried to run those tests on a single thread to double check. On the single thread everything passes. IRL these requests would be rate limited anyways.

That is very cool, about the Prefer: tx=rollback. I think I have some use cases in mind for the future! I really appreciate your work on this project @wolfgangwalther, and your attentiveness to supporting it.

[wolfgangwalther]

Fixed a few things, right now the biggest offender is this:

postgrest/test/spec/Feature/Query/ComputedRelsSpec.hs

Lines 107 to 146 in 01a56db

	it "applies data representations to response" $ do
	-- A smoke test for data reps in the presence of computed relations.
	-- The data rep here title cases the designer name before presentation. So here the lowercase version will be saved,
	-- but the title case version returned. Pulling in a computed relation should not confuse this.
	request methodPatch "/designers?select=name,videogames:computed_videogames(name)&id=eq.1"
	[("Prefer", "return=representation"), ("Prefer", "tx=commit")]
	[json| {"name": "sidney k. meier"} |]
	`shouldRespondWith`
	[json|[{"name":"Sidney K. Meier","videogames":[{"name":"Civilization I"}, {"name":"Civilization II"}]}]|]
	{ matchStatus = 200 }
	-- Verify it was saved the way we requested (there's no text data rep for this column, so if we select with the wrong casing, it should fail.)
	get "/designers?select=id&name=eq.Sidney%20K.%20Meier"
	`shouldRespondWith`
	[json|[]|]
	{ matchStatus = 200, matchHeaders = [matchContentTypeJson] }
	-- But with the right casing it works.
	get "/designers?select=id,name&name=eq.sidney%20k.%20meier"
	`shouldRespondWith`
	[json|[{"id": 1, "name":"Sidney K. Meier"}]|]
	{ matchStatus = 200, matchHeaders = [matchContentTypeJson] }
	-- Most importantly, if you read it back even via a computed relation, the data rep should be applied.
	get "/videogames?select=name,designer:computed_designers(*)&id=eq.1"
	`shouldRespondWith`
	[json|[
	{"name":"Civilization I","designer":{"id": 1, "name":"Sidney K. Meier"}}
	]|] { matchHeaders = [matchContentTypeJson] }
	-- reset the test fixture
	request methodPatch "/designers?id=eq.1"
	[("Prefer", "tx=commit")]
	[json| {"name": "Sid Meier"} |]
	`shouldRespondWith` 204
	-- need to poke the second one too to prevent inherent ordering from changing
	request methodPatch "/designers?id=eq.2"
	[("Prefer", "tx=commit")]
	[json| {"name": "Hironobu Sakaguchi"} |]
	`shouldRespondWith` 204