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

Proposal: Automatically reorder foreign key constraint statements #405

Closed
hokaccha opened this issue May 23, 2023 · 5 comments · Fixed by #500
Closed

Proposal: Automatically reorder foreign key constraint statements #405

hokaccha opened this issue May 23, 2023 · 5 comments · Fixed by #500
Labels
psqldef Bugs or feature requests related to PostgreSQL

Comments

@hokaccha
Copy link
Collaborator

Platform

  • OS: macOS
  • RDBMS: PostgreSQL
  • Version: 0.15.27

--export output

-- No table exists --

Input SQL

CREATE TABLE posts (
  id      bigint PRIMARY KEY,
  user_id bigint
);

ALTER TABLE ONLY public.posts ADD CONSTRAINT posts_ibfk_1 FOREIGN KEY (user_id) REFERENCES users (id);

CREATE TABLE users (
  id   bigint PRIMARY KEY,
  name text
);

Current output

ALTER TABLE is performed before create table 'public.users': 'ALTER TABLE ONLY public.posts ADD CONSTRAINT posts_ibfk_1 FOREIGN KEY (user_id) REFERENCES users (id)'

Expected output

CREATE TABLE posts (
  id      bigint,
  user_id bigint
);
CREATE TABLE users (
  id   bigint,
  name text
);
ALTER TABLE ONLY public.posts ADD CONSTRAINT posts_ibfk_1 FOREIGN KEY (user_id) REFERENCES users (id);

Currently, with sqldef, you need to consider the order of statements when adding foreign keys. Simply moving the addition of the foreign key below the creation of the users table would work, but having constraints related to the posts table near the posts table improves readability.

Also, when the DDL becomes large, you may want to split the file.

-- schema/posts.sql
CREATE TABLE posts (
  id      bigint PRIMARY KEY,
  user_id bigint
);

ALTER TABLE ONLY public.posts ADD CONSTRAINT posts_ibfk_1 FOREIGN KEY (user_id) REFERENCES users (id);
-- schema/users.sql
CREATE TABLE users (
  id   bigint PRIMARY KEY,
  name text
);

In this case, you need to consider the order when merging the schema, but ideally, you want to execute it without considering the order, like this:

$ cat schema/*.sql | psqldef --dry-run --host localhost --user postgres sandbox

Proposal

To solve this problem, I propose a change in the sqldef generator to place the DDL for adding foreign keys after all create table statements.

For reference, ridgepole behaves in the same way.

create_table "posts", force: :cascade do |t|
  t.bigint "user_id"
end

add_foreign_key "posts", "users", name: "posts_ibfk_1"

create_table "users", force: :cascade do |t|
  t.text   "name"
end
$ bundle exec ridgepole -c config.yml --apply --dry-run
Apply `Schemafile` (dry-run)
create_table("posts", **{}) do |t|
  t.column("user_id", :"bigint", **{})
end

create_table("users", **{}) do |t|
  t.column("name", :"text", **{})
end

add_foreign_key("posts", "users", **{:name=>"posts_ibfk_1"})

# CREATE TABLE "posts" (
# "id" bigserial primary key,
# "user_id" bigint)
# CREATE TABLE "users" (
# "id" bigserial primary key,
# "name" text)
# ALTER TABLE "posts" ADD CONSTRAINT "posts_ibfk_1"
# FOREIGN KEY (
# "user_id")
#   REFERENCES "users" ("id")
$ bundle exec ridgepole -c config.yml --apply 
Apply `Schemafile`
-- create_table("posts")
   -> 0.0039s
-- create_table("users")
   -> 0.0037s
-- add_foreign_key("posts", "users", {:name=>"posts_ibfk_1"})
   -> 0.0053s

Are there any design issues with this change in sqldef? If there are no problems, I will attempt to create a Pull Request.

@k0kubun
Copy link
Collaborator

k0kubun commented May 25, 2023

To solve this problem, I propose a change in the sqldef generator to place the DDL for adding foreign keys after all create table statements.
Are there any design issues with this change in sqldef?

The design sounds good to me.

If there are no problems, I will attempt to create a Pull Request.

Looking forward to it. Since you already seem to understand the core implementation of psqldef and I trust you, I'd like to make you a co-maintainer of sqldef if you're interested. WDYT?

@hokaccha
Copy link
Collaborator Author

Since you already seem to understand the core implementation of psqldef and I trust you, I'd like to make you a co-maintainer of sqldef if you're interested. WDYT?

Thank you for considering me! I'd be happy to help out as a co-maintainer of sqldef. Let's do this!

@k0kubun
Copy link
Collaborator

k0kubun commented May 26, 2023

Thank you! Invited.

@k0kubun
Copy link
Collaborator

k0kubun commented May 28, 2023

FYI, pushing a git tag allows you to release a new version. I generally just release every pull request as a new version, so please feel free to do the same.

@hokaccha
Copy link
Collaborator Author

I got it, thank you!

@k0kubun k0kubun added the psqldef Bugs or feature requests related to PostgreSQL label Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
psqldef Bugs or feature requests related to PostgreSQL
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants