Account for schema when checking whether a column exists - #505
Merged
Conversation
brandur
force-pushed
the
brandur-handle-schema
branch
from
August 6, 2024 00:22
3a776cf to
227cf7b
Compare
bgentry
approved these changes
Aug 6, 2024
| _, err = exec.Exec(ctx, "CREATE SCHEMA another_schema_123") | ||
| require.NoError(t, err) | ||
|
|
||
| _, err = exec.Exec(ctx, "SET search_path = another_schema_123") |
Contributor
There was a problem hiding this comment.
Does this pollute the search path for this connection for future tests? Or does the conn pool get thrown away entirely after each test before the test DB goes back into the DB pool?
Contributor
Author
There was a problem hiding this comment.
It gets rolled back with the transaction:
river_dev=# show search_path;
search_path
---------------------------------------------
"$user", infra, logs, public
(1 row)
river_dev=# begin;
BEGIN
river_dev=*# set search_path = 'hello';
SET
river_dev=*# show search_path;
search_path
-------------
hello
(1 row)
river_dev=*# rollback;
ROLLBACK
river_dev=# show search_path;
search_path
---------------------------------------------
"$user", infra, logs, public
(1 row)This one's aimed at addressing #503, in which migrations won't run when using an alternate schema with `search_path` because the `ColumnExists` query isn't accounting for schema in any way. Here, modify `ColumnExists` so that it looks up within `CURRENT_SCHEMA`, which will respect search path. Add test assertions to verify that this works as expected, and similar ones in `TableExists` to avoid any regressions there (it already does respect search path as written, but a future change could potentially break that). Fixes #503.
brandur
force-pushed
the
brandur-handle-schema
branch
from
August 6, 2024 00:45
227cf7b to
38f9fc5
Compare
Merged
brandur
added a commit
that referenced
this pull request
Aug 6, 2024
Merged
brandur
added a commit
that referenced
this pull request
Aug 9, 2024
tigrato
pushed a commit
to gravitational/river
that referenced
this pull request
Dec 18, 2024
Prepare release v0.11.2, containing riverqueue#504 and riverqueue#505. This will also require a separate CLI release because of some minimum Go version changes in all the project's `go.mod` files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This one's aimed at addressing #503, in which migrations won't run when
using an alternate schema with
search_pathbecause theColumnExistsquery isn't accounting for schema in any way.
Here, modify
ColumnExistsso that it looks up withinCURRENT_SCHEMA,which will respect search path. Add test assertions to verify that this
works as expected, and similar ones in
TableExiststo avoid anyregressions there (it already does respect search path as written, but a
future change could potentially break that).
Fixes #503.