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

sql/postgres: fixed inspection/migration for enums and indexes #711

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

Conversation

svstanev
Copy link
Contributor

@svstanev svstanev commented Apr 16, 2022

I'm evaluating Atlas for a project I'm working on. We're using Postgres and I stumbled across a couple of issues related to enums and indexes.

Enums:

  1. When inspecting a database where 2 or more columns use the same enum type result contains enum definitions for each column which results in duplicates in the Atlas schema and later in the change plan/migrations;
  2. schema.EnumType lacks schema information which is important for Pg as user types belong to a schema (namespace). This is fatal when performing NormalizeRealm: the schema name is changed to a temp one (atlas_dev_%s_%d) and then applied to the dev db. Since enums don't have a reference to the schema.Schema they belong to, the generated schema change statements (sql) are incorrect as enums are referenced by name only which usually means that they are created in the public schema and not the atlas_dev_xxx schema. Also enum types may be referenced in expressions like column default value:
create table if not exists table1 (
  ...
  state state_enum not null default 'on'::state_enum,
  ...
);

that should also reference the enum by the correct schema-qualified name.

Indexes:

1. I have an unique index like this:

```sql
create unique index if not exists u_entity_name on entity (
  entity_type,
  coalesce(parent_id,0),
  name
);

which when inspected become like this:

index "u_entity_name" {
    unique = true
    type   = BTREE
    on {
      expr = "COALESCE(parent_id, (0)::bigint)"
    }
    on {
      expr = "COALESCE(parent_id, (0)::bigint)"
    }
    on {
      expr = "COALESCE(parent_id, (0)::bigint)"
    }
  }

The reason is the query the index metadata is retrieved from Postgres.

Last but not least (as it is the first problem I hit): there is a problem if the DSN contains an username with _: the initial url is split in provider (postgres/...) and dns (the rest after the ://) and the later is parsed as an url (url.Parse). As this url already have the scheme stripped it becomes somewhat malformed as per the standard lib source code and results in an error: first path segment in URL cannot contain colon.

In any case Atlas is amazing and I'm looking forward to start using it as the main schema management tool for the project I'm currently working on.

@a8m
Copy link
Member

a8m commented Apr 16, 2022

Hey @svstanev 👋
Thanks for the kind words and for this amazing contribution.

Do you mind breaking this PR into 3 (URL fix, enums, and indexes)? It will make the review simpler and keep the history cleaner. 🙏

@svstanev
Copy link
Contributor Author

@a8m I should have done it the first time but I discovered the index problem while debugging the enums and fixed both simultaneously. Anyway I'll submit the new PRs shortly.

@masseelch masseelch requested review from masseelch and removed request for masseelch May 13, 2022 08:28
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