Forums | Mahara Community

Developers /
Upgrade code for notification_internal_activity seems completely broken


Howard Miller's profile picture
Posts: 191

29 August 2011, 8:34

I'm talking about the code in lib/db/upgrade.php for < 2010042600 which adds some fields and constraints to notification_internal_activity.  I'm prepared to accept that I'm going mad but it seems completely broken in MySQL. Because...

* It adds the field 'from'. From is a reserved word in MySQL. I know that Mahara checks for reserved words and escapes them but.... (a) 'from' is not in the list of reserved words and (b) even if it was the correct quote character is the back-tick not the double-quote as used in Mahara. (see http://dev.mysql.com/doc/refman/5.5/en/identifiers.html)

* I put some debugging code in the add_key() code (in lib/ddl.php) to see what it was trying to do. It creates two commands - one to add the index and the other to create the constraint. It looks like it has created them in the wrong order. Here is the  $sqlarr array contents just before it goes off to get executed...

 [0] => ALTER TABLE `notification_internal_activity` ADD CONSTRAINT `notiinteacti_fro_fk` FOREIGN KEY (`from`) REFERENCES `usr` (`id`)
[1] => CREATE INDEX `notiinteacti_fro_ix` ON `notification_internal_activity` (`from`)

...which looks to me as though it is trying to add the constraint before it adds the index. My understanding is that will fail as the index is a prerequisite for the constraint? BTW... I had previously modified the code to quote everything with backticks. 

For some reason this just fails completely silently....   

Iñaki Arenaza's profile picture
Posts: 253

29 August 2011, 12:46

Sorry to ask the obvious but, have you reported this in the tracker? Wink

Saludos.
Iñaki.

Howard Miller's profile picture
Posts: 191

29 August 2011, 16:19

Working up to it :)

I'm still investigating a nasty upgrade problem with MySQL. As I've said elsewhere, I'm really coming to the worrying conclusion that Mahara is not well supported for MySQL with 1.3 and newer :(

François Marier's profile picture
Posts: 411

29 August 2011, 17:56

I'm surprised to see this because we do actually test fresh installs and upgrades both on Postgres and MySQL before every stable release.

(In addition, while most developers are using Postgres, I've switched my main development environment to MySQL to give it a bit more testing.)

anonymous profile picture
Account deleted
Posts: 808

29 August 2011, 19:02

The trouble is, if a foreign key doesn't get created and there's no error message, most likely we just won't notice.

We should really compare the schemas from a fresh install and an upgrade before every release.  That probably would've caught this one.

Howard Miller's profile picture
Posts: 191

30 August 2011, 13:51

To be honest... this site started life as a pre-release hack that Nigel put together for us back in the day. Against my advice, the "powers that be" insisted that we went to production with this pilot version because they didn't want to loose the data. I suspect our chickens are coming home to roost.

Tomorrow, I'll do a schema compare just to satisfy myself one way or another. 

There's still something weird with the exception handling in Mahara that I neither like not understand. IMO, a database error should simply be caught and it should stop. If people can't take backups then that's their problem surely??

anonymous profile picture
Account deleted
Posts: 808

30 August 2011, 17:27

Howard,

If you're a glutton for punishment, and you want to get a head start on your 1.5 upgrade, we need someone with a large MySQL db to test and rewrite some slow upgrades:

  https://bugs.launchpad.net/mahara/+bug/817796

Ok, so I'm being a bit cheeky, I just thought you might be keen to take that one on while you're in the mood :)

R.

Howard Miller's profile picture
Posts: 191

31 August 2011, 7:41

I know you didn't ask for this, but just to give you a flavor. This is the patch file resulting from comparing schemas of my version of 1.2.10 with a clean install of 1.2.10.

http://mahara.org/artefact/file/download.php?file=159726 

Doesn't make the slightest difference to my original upgrade problem. That's still just as broken :( Looks like I'm stuck on 1.2.

I'll have a go with the 1.5 one for you if we ever get that far :)

anonymous profile picture
Account deleted
Posts: 808

31 August 2011, 17:45

Interesting, it looks like we have a few missing indexes & foreign keys to sort out, and a bunch of inconsistencies in index naming.

Have you tried to upgrade to somewhere just before that notification_internal_activity upgrade was added?  Does it finish successfully if you check out the code at say commit d506c719d0f3916b57559e1cf088a8184ddbed80 and upgrade to there?

Howard Miller's profile picture
Posts: 191

01 September 2011, 4:12

Yes, upgrade to 1.3.0dev worked fine. I had synched the 1.2.10 schema before I started and there are already a few problems creeping in against a clean 1.3.0dev:

ALTER TABLE `artefact` MODIFY COLUMN `allowcomments` tinyint(1) NOT NULL DEFAULT '0' AFTER `authorname`;
ALTER TABLE `artefact_resume_educationhistory` MODIFY COLUMN `institutionaddress` text NULL AFTER `institution`;
ALTER TABLE `artefact_resume_employmenthistory` MODIFY COLUMN `employeraddress` text NULL AFTER `employer`;
ALTER TABLE `site_data` MODIFY COLUMN `ctime` datetime NOT NULL FIRST;
ALTER TABLE `view` MODIFY COLUMN `submittedtime` datetime NULL AFTER `submittedhost`, MODIFY COLUMN `theme` varchar(255) NULL AFTER `layout`, MODIFY COLUMN `visits` bigint(10) NOT NULL DEFAULT '0' AFTER `type`, MODIFY COLUMN `allowcomments` tinyint(1) NOT NULL DEFAULT '1' AFTER `visits`;
ALTER TABLE `view_visit` MODIFY COLUMN `view` bigint(10) NOT NULL FIRST;

I'll see where I can get next :)

14 results