Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow using cascade when truncating tables #702

Merged
merged 4 commits into from
Jul 11, 2021

Conversation

jaitaiwan
Copy link
Contributor

In cases where a data in a database has foreign key references against
the table we're wanting to truncate, we can use the CASCADE key to
ensure that all data referencing the rows we're deleting are also
deleted to ensure the database keeps it's integrity.

  • Added in-code documentation as per crystal spec for the truncate
    command
  • Added new cascade flag to truncate method, set to "false" by default
    (potentially dangerous)
  • Added migration ensuring foreign key constraint on posts by comments
  • Added specification to test cascade deletes data referenced by
    foreign keys

Fixes #648

Some possible considerations:

  • Perhaps we should turn on cascade by default considering it's potentially a dangerous state to truncate ignoring foreign key constraints? However I know that cascade by default would delete even more data so perhaps that's unwarranted
  • Technically speaking the spec I added is really only testing if Postgres is doing a cascading delete, I'd feel much more comfortable if the resulting ExecResult was inspectable to see what the final query that was run was so that I could do an assertion against it.

In cases where a data in a database has foreign key references against
the table we're wanting to truncate, we can use the CASCADE key to
ensure that all data referencing the rows we're deleting are also
deleted to ensure the database keeps it's integrity.

 - Added in-code documentation as per crystal spec for the truncate
   command
 - Added new cascade flag to truncate method, set to "false" by default
   (potentially dangerous)
 - Added migration ensuring foreign key constraint on posts by comments
 - Added specification to test cascade deletes data referenced by
   foreign keys

 Fixes luckyframework#648
Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

Sweet! Great job on this. I think you just need to run the formatter crystal tool format spec src, and make sure ameba is happy ./bin/ameba. I made a few suggestion, but once those are done, I think this will be good!

src/avram/queryable.cr Outdated Show resolved Hide resolved
src/avram/queryable.cr Outdated Show resolved Hide resolved
Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

Sweet! Thanks for the contribution.

@jwoertink jwoertink merged commit 98ed8b7 into luckyframework:master Jul 11, 2021
@jaitaiwan jaitaiwan deleted the f/add_cascade_to_truncate branch July 12, 2021 02:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a CASCADE option to TRUNCATE
2 participants