search

skeema / tengo

Go La Tengo: a MySQL automation library
Apache License 2.0
27 stars 19 forks source link

Foreign Key Constraint Support #1

Closed chrisjpalmer closed 7 years ago

chrisjpalmer commented 7 years ago

This pull request enables foreign key constraint support for the tengo library. It includes support for cross schema constraints and correctly handles constraints with the RESTRICT mode.

chrisjpalmer commented 7 years ago

Remembered to put it into the foreign keys branch!

chrisjpalmer commented 7 years ago

Hi Evan. I have completed the tests for constraints and also made an amendment to the the Constraint struct. It contained ColumnName string and I changed this to be Column *Column so it is more consistent with the rest of the application. I have performed some testing on a bunch of databases which I administer and it seems to be working very well. Obviously you would want to perform your own testing to verify that everything works.

Looking forward to hearing from you.

Chris

evanelias commented 7 years ago

Awesome, this is great, thanks again Chris! I'll review and test this weekend.

On Tue, Jul 25, 2017 at 11:18 PM, Chris Palmer notifications@github.com wrote:

Hi Evan. I have completed the tests for constraints and also made an amendment to the the Constraint struct. It contained ColumnName string and I changed this to be Column *Column so it is more consistent with the rest of the application. I have performed some testing on a bunch of databases which I administer and it seems to be working very well. Obviously you would want to perform your own testing to verify that everything works.

Looking forward to hearing from you.

Chris

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/skeema/tengo/pull/1#issuecomment-317935975, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEY8E3ZMh05WG4E-j_BgAX84xRq2pnuks5sRrANgaJpZM4OgqlX .

evanelias commented 7 years ago

Functionality looks perfect! Thanks for adding the unit tests too, this is excellent.

I'm going to go ahead and merge this into the foreign_keys branch, and then add some quick style changes on top in a separate commit. It also looks like some recent README changes were inadvertently reverted here; I'll fix those too. (I think what happened is I updated the README here in f7108f254, but I didn't update the vendored version in skeema/skeema since it was only a documentation change.)

There are a few other things I want to look into on my end before merging foreign_keys into master, and then updating the vendored version in skeema/skeema. It's just the items I mentioned under "Future considerations on my end" in my comment on skeema/skeema#8. Might take a couple more weeks on that, apologies in advance if so -- I'm in the process of switching to a new computer at home and still have a lot of development-related things to set up :)