Skip to content
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

fix/schema-scope-qualifier #2647

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danyloshevchenkoingka
Copy link

@danyloshevchenkoingka danyloshevchenkoingka commented Mar 27, 2024

The PR adds a no-schema flag so that the generated migrations don't include a schema qualifier. This allows to generate cross-schema migrations

Resolves this issue: #1625

@a8m
Copy link
Member

a8m commented Mar 27, 2024

Thanks for the PR, @danyloshevchenkoingka. However, the "no-schema" behavior is detected from the database URL. Please, read about this in our website: https://atlasgo.io/concepts/url#scope

@danyloshevchenkoingka
Copy link
Author

danyloshevchenkoingka commented Mar 27, 2024

Thanks for the PR, @danyloshevchenkoingka. However, the "no-schema" behavior is detected from the database URL. Please, read about this in our website: atlasgo.io/concepts/url#scope

When I generate migrations for a postgres database limited to a schema scope by specifying parameters
--dev-url "postgres://postgres:[email protected]:5432/postgres?search_path=dev_schema&sslmode=disable" and
--to "postgres://user:pass@host:5432/db?search_path=schema_target_name

the generated results include CREATE SCHEMA "schema_target_name"; statement and the CREATE TABLE "schema_target_name"."table_name" statements include the target schema qualifier.

@a8m
Copy link
Member

a8m commented Mar 27, 2024

Please paste the full command.

When working in schema scope, no CREATE SCHEMA commands should be generated and the qualifier should not be before the objects as well.

@danyloshevchenkoingka
Copy link
Author

danyloshevchenkoingka commented Mar 27, 2024

Please paste the full command.

$ atlas migrate diff init_db --dev-url "postgres://postgres:[email protected]:5432/postgres?search_path=dev_schema&sslmode=disable" --to "postgres://user:pass@host:5432/itpgdb?search_path=iwar03&sslmode=disable" --dir-format golang-migrate --format '{{ sql . "    " }}'

@a8m
Copy link
Member

a8m commented Mar 27, 2024

I wasn't able to reproduce this:

➜  atlas migrate diff \             
--dev-url "postgres://postgres:pass@localhost:5432/test?search_path=dev&sslmode=disable" \
--to "postgres://postgres:pass@localhost:5432/test?search_path=public&sslmode=disable" \
--format '{{ sql . "    " }}'
The migration directory is synced with the desired state, no changes to be made
➜  docker exec -it 34be7175d278 psql -U postgres
psql (15.5 (Debian 15.5-1.pgdg110+1))
Type "help" for help.

postgres=# \c test;
You are now connected to database "test" as user "postgres".
test=# create table t(c int);
CREATE TABLE
test=# exit
➜  atlas migrate diff \                         
--dev-url "postgres://postgres:pass@localhost:5432/test?search_path=dev&sslmode=disable" \
--to "postgres://postgres:pass@localhost:5432/test?search_path=public&sslmode=disable" \
--format '{{ sql . "    " }}'  
➜  cat migrations/20240327104858.sql 
-- Create "t" table
CREATE TABLE "t" (
    "c" integer NULL
);

I recommend you to use the docker:// driver for the dev-database instead and see if this solves the issue:

"docker://postgres/15/dev?search_path=public"

@danyloshevchenkoingka
Copy link
Author

I recommend you to use the docker:// driver for the dev-database instead and see if this solves the issue:

$ psql -d postgres -U postgres
psql (16.2 (Homebrew))
Type "help" for help.

postgres=# CREATE DATABASE test_db;
CREATE DATABASE
postgres=# \c test_db
You are now connected to database "test_db" as user "postgres".
test_db=# CREATE SCHEMA test_schema;
CREATE SCHEMA
test_db=# SET search_path TO test_schema;
SET
test_db=# CREATE TABLE test_table (a int, b int);
CREATE TABLE
test_db=# \d
              List of relations
   Schema    |    Name    | Type  |  Owner
-------------+------------+-------+----------
 test_schema | test_table | table | postgres
(1 row)

test_db=#

Execute altas with the docker:// driver:

$ atlas migrate diff create_all --dev-url "docker://postgres/16/dev?search_path=public" --to "postgres://postgres:pass@localhost:5432/test_db?search_path=test_schema&sslmode=disable" --dir-format golang-migrate --format '{{ sql . "    " }}'
Error: *schema.ModifySchema is not allowed when migration plan is scoped to one schema

@danyloshevchenkoingka
Copy link
Author

danyloshevchenkoingka commented Mar 27, 2024

If we look into the code, the condition to skip the schema qualifier in the function sqlx.Builder.mayQualify is only when the Builder.Schema is non-nil empty string. Otherwise the default schema (which is the target schema) qualifier is used.

switch {
// Custom qualifier.
case b.Schema != nil:
	// Empty means skip prefix.
	if *b.Schema != "" {
		b.Ident(*b.Schema)
		b.rewriteLastByte('.')
	}
// Default schema qualifier.
case s != nil && s.Name != "":
	b.Ident(s.Name)
	b.rewriteLastByte('.')
}

If we specify the schema scope for the dev database, the migrateDiffRun function always sets the option migrate.PlanWithSchemaQualifier(dev.URL.Schema), so the condition above is never met and the qualifier is always set.

I've updated the PR to fix this behaviour.

@danyloshevchenkoingka danyloshevchenkoingka changed the title Add a no-schema flag fix/schema-scope-qualifier Mar 27, 2024
@a8m
Copy link
Member

a8m commented Mar 27, 2024

I recommend you to use the docker:// driver for the dev-database instead and see if this solves the issue:

$ psql -d postgres -U postgres
psql (16.2 (Homebrew))
Type "help" for help.

postgres=# CREATE DATABASE test_db;
CREATE DATABASE
postgres=# \c test_db
You are now connected to database "test_db" as user "postgres".
test_db=# CREATE SCHEMA test_schema;
CREATE SCHEMA
test_db=# SET search_path TO test_schema;
SET
test_db=# CREATE TABLE test_table (a int, b int);
CREATE TABLE
test_db=# \d
              List of relations
   Schema    |    Name    | Type  |  Owner
-------------+------------+-------+----------
 test_schema | test_table | table | postgres
(1 row)

test_db=#

Execute altas with the docker:// driver:

$ atlas migrate diff create_all --dev-url "docker://postgres/16/dev?search_path=public" --to "postgres://postgres:pass@localhost:5432/test_db?search_path=test_schema&sslmode=disable" --dir-format golang-migrate --format '{{ sql . "    " }}'
Error: *schema.ModifySchema is not allowed when migration plan is scoped to one schema

I see what the issue is. The public schema has a default comment setting that is different than your schema. So, my docker:// example is irrelevant in this case. But using another local schema should do the work like I pasted above: #2647 (comment)

@a8m
Copy link
Member

a8m commented Mar 27, 2024

If we specify the schema scope for the dev database, the migrateDiffRun function always sets the option migrate.PlanWithSchemaQualifier(dev.URL.Schema), so the condition above is never met and the qualifier is always set.

The flag is default to empty string. The existing code is fine

@danyloshevchenkoingka
Copy link
Author

The flag is default to empty string

Yes, you're right, sorry.

The public schema has a default comment setting that is different than your schema

Shouldn't we skip the change in this case?

@a8m
Copy link
Member

a8m commented Mar 27, 2024

Shouldn't we skip the change in this case?

I don't think so. This is desired.

I'm not sure I understand the issue, because the commands you gave above simply work for me.

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