Skip to content
This repository has been archived by the owner on Mar 4, 2021. It is now read-only.

Fixing upgrade script failures when a dbprefix is defined in database.php #143

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

abv
Copy link
Contributor

@abv abv commented Aug 2, 2011

I ran into this issue when attempting to upgrade from version 0.90 to version 1. This commit partially addresses issues where OpenVBX's upgrade scripts cannot run because there is a dbprefix set in database.php.

For example if $db["default"]["dbprefix"] = "vbx_", then the SQL in /updates/47.sql UPDATE settingsSETvalue= '0.91' WHEREname = 'version'... won't execute and the upgrade process will halt and throw an error.

This fix required that $db["default"]["swap_pre"] = "{PRE}" be added to databases.php and the string {PRE} be placed before the names of any tables in any raw SQL executed by CodeIgniter. CodeIgniter's DB driver takes care of the dirty work, and swaps the swap_pre string with the db_prefix string for raw query strings. To see how this works, see line 257 of CodeIgniter's DB_driver.php.

I only added the swap_pre string the updates files 47 and up, because those were the only files that affected our upgrade from version 0.90 to 1.0. I didn't think the scenario where someone upgrading from an earlier version would run into this same issue was realistic. I also looked through OpenVBX's code for other instances where there were raw SQL was being passed to $ci->db->query() but didn't encounter any.

I also, defined the swap_pre string in the database-sample.php -- but obviously, for this to work, the swap_pre string would need to be set to "{PRE}" in database.php.

@Gipetto
Copy link
Contributor

Gipetto commented Aug 2, 2011

Cool.
I think this is a good change (re: e3c0753). We'll do a evaluation of the OpenVBX code to see how many places this needs to touch.

@abv
Copy link
Contributor Author

abv commented Aug 3, 2011

Just noticed that you'd also need to add the {PRE}s to openvbx.sql in the root directory. Sorry about this pull request having multiple commits associated with it. If you run a diff on it, the only differences are the ones related the the dbprefix issue. From now on I'll create a new branch with merged with master, so you don't see any unrelated commits.

@Gipetto
Copy link
Contributor

Gipetto commented Sep 27, 2011

I've merged this locally and have started testing. I think that the install process should have an advanced option to select the db-prefix as well so I'll be adding that in while I test.

@Gipetto
Copy link
Contributor

Gipetto commented Sep 27, 2011

There's a fun problem here that may make this hard to do. I've run in to this in the db->query method:

if ( ($this->dbprefix != '' AND $this->swap_pre != '') AND ($this->dbprefix != $this->swap_pre) ) { ... }

That effectively prevents the swap_pre from being processed if there's no dbprefix to swap in, essentially denying us the ability to have optional prefixes. I'm trying to find a way around this without creating too much headache, but I'm not confident that any work around will be without caveats that make creating upgrades harder.

@Gipetto
Copy link
Contributor

Gipetto commented Sep 27, 2011

I'm gonna shelf this for a later update. I think I want to change to using a prefix by default but need to better plan how to manage the upgrade process.

@Gipetto
Copy link
Contributor

Gipetto commented Dec 1, 2011

This one is still a touchy subject as CodeIgniter has some very strict handling around the db-prefix. I'm thinking that the correct course of action here would be to start using a db-prefix in new installs of OpenVBX. All in all its a good idea but, as is the case with something like this, it would require lots of testing.

I still want to do this.

@abv
Copy link
Contributor Author

abv commented Dec 1, 2011

Thanks for keeping this issue alive. Too bad there isn't an easier way to implement this.

@Gipetto
Copy link
Contributor

Gipetto commented Dec 1, 2011

Yeah, I was so close and then CodeIgniter slammed the door shut in my face.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants