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

Use explicit column name for FK references #62

Merged
merged 1 commit into from
Aug 24, 2024

Conversation

crbelaus
Copy link
Contributor

The ErrorTracker tables use id as the primary key name. This was causing trouble when the client repository was configured to use a different column name for references since it tried to create a foreign key pointing to a non-existing column.

This commit explicitly states the referenced column name to avoid this problems.

To check this you can create a new Phoenix application that uses the error tracker and add the following configuration to the application repository:

config :my_app, MyApp.Repo,
  migration_primary_key: [name: :uuid, type: :binary_id],
  migration_foreign_key: [column: :uuid, type: :binary_id]

Trying this on main will raise an error when running the migrations. The error is not present in this branch.

Closes #59

The ErrorTracker tables use `id` as the primary key name. This was
causing trouble when the client repository was configured to use a
different column name for references since it tried to create a foreign
key pointing to a non-existing column.

This commit explicitly states the referenced column name to avoid this
problems.
@crbelaus crbelaus requested a review from odarriba August 22, 2024 16:56
@crbelaus crbelaus self-assigned this Aug 22, 2024
@crbelaus
Copy link
Contributor Author

It is worth mentioning that this fails only on Postgres. SQLite creates the column but ignores the FK creation if there are any errors. I've updated both migrations regardless to ensure that they are consistent.

@petrus-jvrensburg
Copy link

Thanks @crbelaus !
I tried this branch locally, and it's working as expected.

Copy link
Contributor

@odarriba odarriba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch 🚀

@odarriba odarriba merged commit e5eea0b into main Aug 24, 2024
3 checks passed
@odarriba odarriba deleted the explicit-column-name-for-references branch August 24, 2024 12:02
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.

Migration error during setup
3 participants