Conversation
Signed-off-by: Abhinav Gyawali <22275402+abhizer@users.noreply.github.com>
snkas
left a comment
snkas
left a comment
There was a problem hiding this comment.
Could you briefly mention the reason ("why") these changes are necessary in the PR description? I assume that the same table name is being used in different tests, which are run concurrently, thus potentially leading to conflicts?
|
There was a failure because the tables were deleted. It could be because of conflict, it could also be because we were writing too slow to Postgres because the buffer size is only 1024 and each record is about 600 bytes. |
mythical-fred
left a comment
mythical-fred
left a comment
There was a problem hiding this comment.
Clean targeted fix: UUID-suffixed table/publication names eliminate cross-run aliasing on a shared CI Postgres, the CREATE TYPE becomes idempotent via DO $$ ... EXCEPTION WHEN duplicate_object, and the 40s→120s timeout matches what the integration runner has actually been wanting. Snkas already covered the bulk. Two non-blocking nits below.
Worth filing as a tiny follow-up: the shared test_struct is now write-once — if its shape ever needs to change, the EXCEPTION WHEN duplicate_object swallow will silently keep the old definition. A DROP TYPE IF EXISTS test_struct CASCADE guarded by a schema-version probe (or just a versioned type name test_struct_v1) would future-proof it. Not for this PR.
| "config": { | ||
| "uri": url, | ||
| "table": table_name, | ||
| "table": &table_name, |
There was a problem hiding this comment.
Two-space indent regression: "table": &table_name, is misaligned with the surrounding object keys. Also at the equivalent spot in test_pg_upsert further down.
| let max_prefix_len = 63 - suffix.len() - 1; | ||
| let prefix = &prefix[..prefix.len().min(max_prefix_len)]; | ||
|
|
||
| format!("{prefix}_{suffix}") |
There was a problem hiding this comment.
&prefix[..prefix.len().min(max_prefix_len)] will panic if prefix ever contains multi-byte UTF-8 and the cut lands inside a code point. Today every caller passes an ASCII literal so this is theoretical, but prefix.char_indices().nth(max_prefix_len) (or asserting ASCII) would make it bullet-proof. Drive-by.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
3 participants
Just updates the postgres tests to use unique table names and increases the timeout.