important reminder: do not use API functions in update code
Hey, As more and more schema API functionality is added to Drupal, and as more and more database changes are made in each release, it is very important for you to remenber this: **Avoid using API functions (ie. anything used in the Drupal runtime too) in your update functions** There were several instances of this before. Previous updates used node content type listing API functions, some of which were simply gone later, so no update-hopping (through multiple versions) was possible without some update code hacking. But it is just as easy to write bad code when in the same Drupal version. Take system_update_6027 for example, which we try to fix at http://drupal.org/node/197500 . - it calls _block_rehash() (note the underscore, not even a public API function!) - _block_rehash() uses drupal_save_record(), which uses the most up to date schema from system_schema() Now what happens if the schema is changed in a later update in Drupal 6? (ie. > 6027 but < 7000). Well, system_update_6027() becomes broken, because its _block_rehash() call tries to use the latest schema (as in after all Drupal 6 updates are run), but all Drupal 6 updates are not run yet. Think about this when you are doing your Drupal 6 upgrade functions for your modules. Gabor
So all modules should still use db_query for updates (as in 5.x) or does this only apply to core? Gábor Hojtsy wrote:
Hey,
As more and more schema API functionality is added to Drupal, and as more and more database changes are made in each release, it is very important for you to remenber this:
**Avoid using API functions (ie. anything used in the Drupal runtime too) in your update functions**
There were several instances of this before. Previous updates used node content type listing API functions, some of which were simply gone later, so no update-hopping (through multiple versions) was possible without some update code hacking. But it is just as easy to write bad code when in the same Drupal version. Take system_update_6027 for example, which we try to fix at http://drupal.org/node/197500 .
- it calls _block_rehash() (note the underscore, not even a public API function!) - _block_rehash() uses drupal_save_record(), which uses the most up to date schema from system_schema()
Now what happens if the schema is changed in a later update in Drupal 6? (ie. > 6027 but < 7000). Well, system_update_6027() becomes broken, because its _block_rehash() call tries to use the latest schema (as in after all Drupal 6 updates are run), but all Drupal 6 updates are not run yet.
Think about this when you are doing your Drupal 6 upgrade functions for your modules.
Gabor
-- Sean Robertson Web Developer NGP Software, Inc. seanr@ngpsoftware.com (202) 686-9330 http://www.ngpsoftware.com
On Dec 4, 2007 3:44 PM, Sean Robertson <seanr@ngpsoftware.com> wrote:
So all modules should still use db_query for updates (as in 5.x) or does this only apply to core?
How is this related? It depends. When you need to update thousands of items one by one, you are better doing db_query(). With results going to the update screen, use update_sql(). This is the de-facto way for now as far as I see. We will see whether the update_sql() or db_query() syntax changes in Drupal 7, which will break all update functions too. Gabor
On Dec 4, 2007 3:29 PM, Gábor Hojtsy <gabor@hojtsy.hu> wrote:
Take system_update_6027 for example, which we try to fix at http://drupal.org/node/197500 .
- it calls _block_rehash() (note the underscore, not even a public API function!) - _block_rehash() uses drupal_save_record(), which uses the most up to date schema from system_schema()
Now what happens if the schema is changed in a later update in Drupal 6? (ie. > 6027 but < 7000). Well, system_update_6027() becomes broken, because its _block_rehash() call tries to use the latest schema (as in after all Drupal 6 updates are run), but all Drupal 6 updates are not run yet.
A similar example is system_update_1005() which is still in system.install in the name of letting enterprising folks try to update their 4.7 sites to 6.0. Well, it uses some API functions, namely: _node_type_set_defaults node_type_save cache_clear_all system_modules menu_rebuild node_types_rebuild (db_ functions and variable_ functions are mostly taken to be usable in updates). So guess which one of the above breaks in a Drupal 6. Out of pure luck, only menu_rebuild() (at least in my testing), as at system_update_1005(), the menu system is far from being in the state where it is expected in Drupal 6. So why isn't this a critical bug? Well, this is in pre 5.0 update code, and we officially only support updates from 5.x. So why are pre 5.x update functions still in Drupal 6? Nobody suggested to remove them yet (although we removed pre 4.7 update code). Let's discuss! Gabor
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Gábor Hojtsy schrieb:
On Dec 4, 2007 3:29 PM, Gábor Hojtsy <gabor@hojtsy.hu> wrote:
Take system_update_6027 for example, which we try to fix at http://drupal.org/node/197500 .
- it calls _block_rehash() (note the underscore, not even a public API function!) - _block_rehash() uses drupal_save_record(), which uses the most up to date schema from system_schema()
Now what happens if the schema is changed in a later update in Drupal 6? (ie. > 6027 but < 7000). Well, system_update_6027() becomes broken, because its _block_rehash() call tries to use the latest schema (as in after all Drupal 6 updates are run), but all Drupal 6 updates are not run yet.
A similar example is system_update_1005() which is still in system.install in the name of letting enterprising folks try to update their 4.7 sites to 6.0. Well, it uses some API functions, namely:
_node_type_set_defaults node_type_save cache_clear_all system_modules menu_rebuild node_types_rebuild
(db_ functions and variable_ functions are mostly taken to be usable in updates).
So guess which one of the above breaks in a Drupal 6. Out of pure luck, only menu_rebuild() (at least in my testing), as at system_update_1005(), the menu system is far from being in the state where it is expected in Drupal 6.
So why isn't this a critical bug? Well, this is in pre 5.0 update code, and we officially only support updates from 5.x. So why are pre 5.x update functions still in Drupal 6? Nobody suggested to remove them yet (although we removed pre 4.7 update code). Let's discuss!
I am in favour of dropping the code for version-hopping updates. It has always been a PITA to maintain it. If we leave it in, we need to fix it, though. Let's simply remove it. Cheers, Gerhard -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFHVXzlfg6TFvELooQRAkG6AKDAV7EhHqQmNarlME3uCDDSstwzeACgioV8 4hDrF+o4oonafUYu603muqM= =8kkY -----END PGP SIGNATURE-----
On Dec 4, 2007 5:14 PM, Gerhard Killesreiter <gerhard@killesreiter.de> wrote:
I am in favour of dropping the code for version-hopping updates. It has always been a PITA to maintain it. If we leave it in, we need to fix it, though. Let's simply remove it.
OK, that avoids some of the problems (anyone up for an issue). The original note still applies to anyone writing updates. Gabor
On Dec 4, 2007 5:14 PM, Gerhard Killesreiter <gerhard@killesreiter.de> wrote:
I am in favour of dropping the code for version-hopping updates. It has always been a PITA to maintain it. If we leave it in, we need to fix it, though. Let's simply remove it.
OK, that avoids some of the problems (anyone up for an issue).
The original note still applies to anyone writing updates.
Gabor
Just curious: Wouldn't it make sense to move all _old_ API functions as stub functions into a new update_api.inc (or similar named file) that is only included by update.php? Example: function menu_rebuild() { ...calls to new API functions... } So we wouldn't loose version-hopping and would probably get a nice overview of old API functions along with their new replacements. If properly documented with phpdoc blocks, this *could* be a neat replacement for our module update handbook pages. sun
On Dec 4, 2007 5:07 PM, Daniel F. Kudwien <news@unleashedmind.com> wrote:
Just curious: Wouldn't it make sense to move all _old_ API functions as stub functions into a new update_api.inc (or similar named file) that is only included by update.php?
Example:
function menu_rebuild() { ...calls to new API functions... }
So we wouldn't loose version-hopping and would probably get a nice overview of old API functions along with their new replacements. If properly documented with phpdoc blocks, this *could* be a neat replacement for our module update handbook pages.
Sun, this does not solve anything, as you would still need these versioned by update number. In system_update_77655() the database behind the menu_rebuild() you would call might easily be different to the database behing system_update_77658() - three updates later. Gabor
On Tue, 4 Dec 2007 16:26:41 +0100, "Gábor Hojtsy" <gabor@hojtsy.hu> wrote:
On Dec 4, 2007 5:14 PM, Gerhard Killesreiter <gerhard@killesreiter.de> wrote:
I am in favour of dropping the code for version-hopping updates. It has always been a PITA to maintain it. If we leave it in, we need to fix it, though. Let's simply remove it.
OK, that avoids some of the problems (anyone up for an issue).
The original note still applies to anyone writing updates.
Gabor
Given our relatively short development cycle, it wouldn't surprise me if there are a lot of people trying to do 2-version jumps. 4.7 is still a supported version and will remain so until D6 is released. Actively blocking people from jumping from a supported version (or just barely not supported) to a supported version (or just barely supported) seems, well, rude. :-) Dropping 4.6 and earlier makes sense. I'm all for recommending that people go through D5 first, but there's no need to make a 4.7->6 upgrade effectively impossible. (Or a 5->7 upgrade later, or 6->8, etc.) --Larry Garfield
Given our relatively short development cycle, it wouldn't surprise me if there are a lot of people trying to do 2-version jumps. 4.7 is still a supported version and will remain so until D6 is released. Actively blocking people from jumping from a supported version (or just barely not supported) to a supported version (or just barely supported) seems, well, rude. :-) Dropping 4.6 and earlier makes sense.
Noone suggests that. Each release would keep the upgrade path from the prior one. So 5.0 keeps its update code from 4.7. And D6 keeps its 5.x upgrade. It just means that folks have to download 5.x if they want to upgrade two versions. This is not so onerous, and actually helps them isolate a problem to a given version should some problem arise.
On Dec 4, 2007 5:16 PM, Moshe Weitzman <weitzman@tejasa.com> wrote:
Given our relatively short development cycle, it wouldn't surprise me if there are a lot of people trying to do 2-version jumps. 4.7 is still a supported version and will remain so until D6 is released. Actively blocking people from jumping from a supported version (or just barely not supported) to a supported version (or just barely supported) seems, well, rude. :-) Dropping 4.6 and earlier makes sense.
Noone suggests that. Each release would keep the upgrade path from the prior one. So 5.0 keeps its update code from 4.7. And D6 keeps its 5.x upgrade. It just means that folks have to download 5.x if they want to upgrade two versions. This is not so onerous, and actually helps them isolate a problem to a given version should some problem arise.
Let's discuss this at http://drupal.org/node/197722, as catch was kindly opening an issue for this with a patch ready for review. Gabor
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Moshe Weitzman schrieb:
Given our relatively short development cycle, it wouldn't surprise me if there are a lot of people trying to do 2-version jumps. 4.7 is still a supported version and will remain so until D6 is released. Actively blocking people from jumping from a supported version (or just barely not supported) to a supported version (or just barely supported) seems, well, rude. :-) Dropping 4.6 and earlier makes sense.
Noone suggests that. Each release would keep the upgrade path from the prior one. So 5.0 keeps its update code from 4.7. And D6 keeps its 5.x upgrade. It just means that folks have to download 5.x if they want to upgrade two versions. This is not so onerous, and actually helps them isolate a problem to a given version should some problem arise.
Right. Cheers, Ger»me, rude?«hard -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFHVYvCfg6TFvELooQRApCoAJ0R5r2tegWGr5gEOcTr9CHcUcAxYACgsuOi Re9nF0+73NNuVfY25ifCWuk= =yLUp -----END PGP SIGNATURE-----
participants (6)
-
Daniel F. Kudwien -
Gerhard Killesreiter -
Gábor Hojtsy -
Larry Garfield -
Moshe Weitzman -
Sean Robertson