Add mysqli support to lm_paypal module

Which database are you using?

Currently LM_PayPal only works with MySQL and its installer will warn you if you try to install on a PostGresSQL system.

I ask because one of the error messages you are getting is becase I call the MySQL specific

mysql_insert_id

I'll have to recode to use the Drupal way of doing it but that isn't trivial.

If you are not using MySQL, and somehow managed to create the database tables, this is almost certainly the cause of your problem.

However it is 2 AM here so I wont be able to fix it tonight. But I will be able to work on it tomorrow evening.

...

Now 2.30 AM (I hate bugs so had to continue looking) and I was wondering if you have the mysqli extension enabled and are using to connect to MySQL? I just noticed that my installer allows mysqli as well as mysql but mysql_insert_id will not then work.

Have a look in your site settings.php file at $db_url and see if it begins mysqli://. If it does then could you try using mysql:// instead, until I fix things?

Lee

My apologies for that rather subtle bug and the time you wasted looking for it. For the short term I've patched lm_paypal.install to reject mysqli until I've fixed lm_paypal.module to not use mysql_insert_id

I'm pretty swamped at the moment so for now I've documented the restriction (MySQL only) on the project homepage.

Lee

I'd like to try this module out on a site using mysqli extension. Should I look into creating a patch..?

Also, any plans for drupal 4.7 and 5 branches of the module? I usually grab my modules from a DRUPAL-4-7 cvs checkout.

mysqli wouldn't take very long to do and if you have the time that would be great. Look for mysqli in lm_paypal.install and then look for mysql_insert_id in lm_paypal.module. I think those are the only two places you'll have to make changes.

I do plan to create proper DRUPAL-4-7 and a version 5 branch. Just a bit swamped at the moment (I'm having to deal with lm_paypal stuff at 2.30 am!)

Lee

OK here's the patch on the module and install file.

For existing sites, I think an update script should run, however. It should get the max id from the ipns table and insert into the sequences tables.

This patch includes update sql. It should get tested out on a sandbox site; I have not tested.

One more.. forgot to fix the table schema for new installs.

Well I noticed that drupal core modules have auto_increment columns *and* use the sequences table. Not clear why. so, this version leaves the column definition as-is....

Thanks for your work on this however it does introduce a couple of other potential problems because it uses db_next_id, which is of course what it should be using.

Firstly db_next_id under mysql/mysqli requires LOCK tables access which not all users will have available to them (do a search on Drupal.org for the discussions on this). Secondly on a busy site that code could become a bottle neck as the lock,update,unlock is likely to be slow compared to the current code. Ironically the current mysql only code is faster than db_next_id, but using db_next_id gets us PostgresSQL where it is fast.

This leads to a bit of a dilema. I'd like to use Mark's code but it will at least slow down any existing users and possibly prevent them from using the module is they cannot get LOCK access.

Opinions please!

Lee

I didn't realize db_next_id() would be problematic; it's used by aggregator.module, comment.module, drupal.module, menu.module, node.module, system.install, taxonomy.module, upload.module, and user.module.

You could always create your own lm_paypal_insert_id() function which would call either mysql_insert_id() or mysqli_insert_id() depending on $GLOBALS['db_type']

Any further thoughts on making lm_paypal "officially" compatible with mysqli?

I would vote that db_next_id's presence in core modules trumps other arguments about speed. Drupal has lots about it that could use some help for high performance sites, but that's a different issue. There may be some who use PostgreSQL, but changing to the what core currently does it isn't likely to turn them away from this module.

I'll be applying the patch to my mysqli site.

Btw, for drupal 6, db_next_id() disappears and is replaced by db_last_insert_id(). See http://drupal.org/node/114774#db_last_insert_id

I'll update lm_paypal to Schema API when we move to Drupal 6 - so it should be support by then. On the Drupal 5 front, I'd like some more feedback about possible solutions.

6.x-1.x has been using the Schema system for a long time. 5.x and earlier are no longer supported. There is nothing left to do to address this issue. If there are other mysqli issues, please raise them in new bug reports.