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.

Sign up for GitHub

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

Jump to bottom

Housekeeping, implement better SQLite DDL, resolve pesky bugs #114

Closed
wants to merge 11 commits into from
Closed

Housekeeping, implement better SQLite DDL, resolve pesky bugs #114

wants to merge 11 commits into from

Conversation

aeneasr
Copy link
Member

@aeneasr aeneasr commented Dec 2, 2021 •
edited
Loading

~This PR adds support for DROP COLUMN without the temp table dance as DROP COLUMN is now officially supported in SQLite 🎉 ~ I had to revert this due to implicit implications on foreign keys and indices :/

It also resolves the notorious "null": false issue.

Additionally, this patch adds new functionality to add_column, allowing to define a foreign key when creating a column. This works for all databases and adds support for introducing foreign keys to SQLite schemas after a table has been created (not possible previously):

	ddl := `ALTER TABLE "users" ADD COLUMN "mycolumn" TEXT NOT NULL DEFAULT 'foo' CONSTRAINT projects_subscription_id_fk REFERENCES subscriptions (id, kid) ON DELETE RESTRICT ON UPDATE NO ACTION;`
	schema.schema["users"] = &fizz.Table{}

	res, _ := fizz.AString(`add_column("users", "mycolumn", "string", {"default": "foo", "size": 50, "foreign_key": {
  "table": "subscriptions",
  "columns": ["id", "kid"],
  "name": "projects_subscription_id_fk",
  "on_delete": "RESTRICT",
  "on_update": "NO ACTION"
}})`, sqt)

This PR also upgrade PostgreSQL and CockroachDB and MySQL to supported versions as the versions used previously were no longer maintained.

I have updated all the tests and integration tests also. Yay!

Closes #110
Closes #62

@aeneasr aeneasr requested a review from fasmat December 2, 2021 18:11
@aeneasr aeneasr changed the title Improve sqlite @aeneasr SQLite supports ALTER TABLE .. DROP COLUMN now Dec 2, 2021
@aeneasr aeneasr changed the title @aeneasr SQLite supports ALTER TABLE .. DROP COLUMN now SQLite supports ALTER TABLE .. DROP COLUMN now Dec 2, 2021
@aeneasr aeneasr changed the title SQLite supports ALTER TABLE .. DROP COLUMN now Housekeeping, implement better SQLite DDL, resolve pesky bugs Dec 2, 2021
@aeneasr aeneasr marked this pull request as draft December 2, 2021 20:06
@aeneasr aeneasr removed the request for review from fasmat December 2, 2021 20:06
@aeneasr aeneasr marked this pull request as ready for review December 3, 2021 10:49
@aeneasr aeneasr requested a review from fasmat December 3, 2021 10:49
@aeneasr
Copy link
Member Author

aeneasr commented Dec 3, 2021

Sorry, this PR grew in scope rather quickly. I resolved many bugs though in MySQL and SQLite...

@aeneasr
Support adding foreign key constraints in add_column
f90d9b2 
This patch reverts some of the logic previously introduced for SQLite. It adds new functionality to `add_column`, allowing to define a foreign key when creating a column. This works for all databases and adds support for introducing foreign keys to SQLite schemas after a table has been created (not possible previously).

This patch also upgrade PostgreSQL and CockroachDB and MySQL to supported versions as the versions used previously were no longer maintained.
@aeneasr
Copy link
Member Author

aeneasr commented Dec 3, 2021

I found another bug in SQLite where the internal meta schema was not appropriately updated on rename_column. During review, I think you can ignore the test stubs and just focus on *.go files :)

@aeneasr
Copy link
Member Author

aeneasr commented Dec 3, 2021

And another one, this time it's rename_table, also not correctly updating the meta schema representation.

@sio4
Copy link
Member

sio4 commented Sep 15, 2022

Oh! @aeneasr, do you still working on this PR? I am currently cleaning up all sub-modules for buffalo and just want to know if you are still working on it.

@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment. Otherwise, this will be closed in 7 days.

@github-actions github-actions bot added the stale label Oct 31, 2022
@github-actions
Copy link

github-actions bot commented Nov 8, 2022

This PR was closed because it has been stalled for 45+7 days with no activity.

@github-actions github-actions bot closed this Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fasmat fasmat Awaiting requested review from fasmat

@stanislas-m stanislas-m Awaiting requested review from stanislas-m

@paganotoni paganotoni Awaiting requested review from paganotoni

Assignees
No one assigned
Labels
s: closed stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

change_column with {null: false} executes SQL "DROP NOT NULL"
2 participants
@aeneasr @sio4