issue #1762 : Forge / Add Foreign Keys

[AndrewPodner]

implements ability to add foreign keys in the Database Forge

[narfbg]

This shouldn't be valid only for MySQL (which is the current case) - I could fork and help you out with this one if you like. Also, you're referencing a $foreign_table variable which comes literally from nowhere - the add_foreign_key() method should accept it, as well as an optional name for the key.

[AndrewPodner]

I will take any help I can get. :) On the $foreign_table, the input parameter is an array that gets extracted in line 1021, which is where the foreign_table and other variables come from. I looked at doing the add_foreign_key() method with individual parameters, but since there were 5 of them, it looked a little a messy to me, so I went with the array and documented it with as much detail as I could in the source code and the user guide. But I do not mind switching it back if you think that would be better.

This one is a pretty ambitious PR for me, but I had to try and see if I could make it work. So yes, please feel free to fork it and improve on it. Any help is greatly appreciated.

Do you think that this will be a case of creating a method in each RDMBS's forge individually? I guess now I am thinking to put the protected method in CI_DB_forge blank and the override it in each individual driver. If so, i will go ahead and set that up, and then work on SQL server version as well.

[AndrewPodner]

@narfbg I have been working on this. The plan at this point is to leave a placeholder method in DB forge that will be overridden by each driver's DB forge class.

I have moved the original method into mysql and mysqli drivers. I have also created drivers for postgreSQL and SQL Server.

However, obviously mysql and mysqli are the same, but the SQL Server syntax also works for PostgreSQL.

My question is: should I have this method in each driver, which is essentially going to end up as duplicated code amongst a few drivers, or alternatively should I keep a single method in DB forge that attempts to detect the active driver and choose the correct syntax at that time? Which would be more aligned with how CI is intended to be structured?

I feel like even if the methods are identical, doing it in each driver is probably the most sound way to accomplish it, but I am not 100% sure how you would prefer it to be done for CI.

Thanks!

[narfbg]

There's a standard way to do pretty much everything in SQL, so there surely is a statement that would work for most, if not all drivers. Given that, you can make a base method in the CI_DB_forge class and then override it only in drivers that have different syntax for it.

In this case, what works for SQL Server and PostgreSQL should actually work for all drivers. MySQL just allows you to add foreign keys inline in the CREATE TABLE statement, but that is an extra feature - it doesn't mean that the standard way to do it doesn't work.

[AndrewPodner]

So basically, the template for the language is going to just be CONSTRAINT FOREIGN KEY (log_id) REFERENCES user_logs(id) ON DELETE CASCADE ON UPDATE NO ACTION which should work for pretty much anything.

[AndrewPodner]

@narfbg Is this more of what you had in mind, or did I totally miss the point?

[colonelclick]

Consider that you also need a reliable way to delete foreign keys as well, if you want this to be useful for migrations (need a down as well as an up). In order to that correctly, I think you will need syntax to specifically name the foreign keys, otherwise you may not know the keynote to be able to remove the key reliably.

CONSTRAINT my_unique_user_selected_key_name_here FOREIGN KEY...

The drop command itself could be accomplished fairly easy by adding the DB_forge class a function for drop_foreign_key, almost identical in function to drop_column, and adding another conditional to _alter_table for $alter_type === 'DROP FOREIGN KEY'