- Development - lists.drupal.org

[drupal-devel] [bug] race condition in variable_set in bootstrap.inc
by Jeremy 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/20114 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: Ricetons Reported by: Ricetons Updated by: Jeremy(a)kerneltrap.org Status: patch What keeps another session from doing a DELETE after your session does its SELECT COUNT but before it does its UPDATE? Or, what keeps another session from doing an INSERT after your session does its SELECT COUNT but before it does its INSERT? All you've done is add another query... I think you'll need to use locking to fix this right. Jeremy(a)kerneltrap.org Previous comments: ------------------------------------------------------------------------ April 6, 2005 - 22:53 : Ricetons This bug was exposed by the poormancron module. We can see that the module itself contains no bug. But in the function variable_get(), the function just execute SQL like this: DELETE bar WHERE key="foo" INSERT INTO bar VALUES(foo, bar2ooo) but suppose this: thread A and thread B has executed the DELETE sentence. And they do INSERT at the same time. then an SQL error like this occured: duplicated key 'foo' in table "variables" It's a critical bug. It should be fixed before the time that drupal 4.6.0 released. ------------------------------------------------------------------------ April 8, 2005 - 03:01 : robertDouglass I also experience this on a site that has many different cron cycles ranging from 2 minutes to 6 hours. The query errors which arise (as noted above) are impossible to reproduce through normal testing yet occur fairly (surprisingly) often in the logs. This doesn't just apply to cron jobs but rather to all code that calls variable_set. Setting to CVS since this hasn't been addressed since the issue was opened. ------------------------------------------------------------------------ April 8, 2005 - 08:10 : jhriggs Attachment: http://drupal.org/files/issues/variable_set_update.patch (922 bytes) I'm not sure that there is a "right" way to fix this. In mysql, we could use a REPLACE rather than DELETE/INSERT, but that wouldn't work for pgsql. And, if we are really seeing this type of race, there is no telling which REPLACE would actually make it last and be saved. Instead, we could do a check to see if the row exists and do an UPDATE if it does and an INSERT otherwise; however, this would be prone to the same problem if we are seeing such a race. That is, they may both see that the row does not exist and both attempt to do an INSERT. This will only happen the first time a variable is created, though, which may be more acceptable than what we are seeing now. If the row does already exist, they would both do UPDATES and would "succeed" in the sense that there is no duplicate key error. There is still no telling which one actually sticks, though. The attached patch takes the second approach.

1 0

[drupal-devel] [bug] comment stats don't update after approval in moderation queue
by ankur 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/18215 Project: Drupal Version: 4.5.2 Component: comment.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: ankur Updated by: ankur Status: patch Attachment: http://drupal.org/files/issues/comment.module_5.diff (861 bytes) I don't know if this is by design, but I experimenting with forcing anonymous comments through the comments approval queue. After I logged in as someone with appropriate permissions and marked the comment "published" in the comments approval queue, the link attached to the appropriate node teaser still said "add new comment" rather than "1 comment" or "1 new comment". Here's a patch for it. I believe the diff for the patch is taken against comment.module from 4.5.2. It makes some minor additions to function comment_admin_edit(). -Ankur ankur

6 13

[drupal-devel] [bug] have db_prefix_tables() handle 'default' key not being set
by danielc 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19609 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: danielc Updated by: danielc Status: patch And the patch to issue 19474 (http://drupal.org/node/19474) contains a patch to provide such information to settings.php. danielc Previous comments: ------------------------------------------------------------------------ March 28, 2005 - 16:24 : danielc Attachment: http://drupal.org/files/issues/database.diff (720 bytes) The db_prefix_tables() function in includes/database.inc generates notices if $db_prefix['default'] is not set. This patch fixes that problem. This has a side benefit of making $db_prefix easier to use. ------------------------------------------------------------------------ March 28, 2005 - 17:06 : moshe weitzman nice ... this has been a bit a pimple for a while. ------------------------------------------------------------------------ March 29, 2005 - 10:40 : danielc Attachment: http://drupal.org/files/issues/database2.diff (1.17 KB) I've come up with a new version of the patch that optimizes the speed of the whole function. ------------------------------------------------------------------------ March 29, 2005 - 10:56 : danielc Change Version from "4.5.2" to "cvs". ------------------------------------------------------------------------ March 29, 2005 - 11:01 : danielc Pardon me, I'm testing if this goes the drupal-dev list now that the version is "cvs." ------------------------------------------------------------------------ April 3, 2005 - 07:45 : Dries Any idea of how much faster the second patch is? I don't think it makes a real difference in which case I'd favor the first patch. ------------------------------------------------------------------------ April 3, 2005 - 13:40 : danielc The speed increase is 5%. ------------------------------------------------------------------------ April 3, 2005 - 13:52 : Dries 5% compared to what? What did you measure? ------------------------------------------------------------------------ April 3, 2005 - 23:56 : danielc I tested the db_prefix_tables() function in two configurations. With my first patch applied and with my second patch applied. The second patch is 5% faster than the first. This improvement is the same for both the string and array formats of $db_url. ------------------------------------------------------------------------ April 7, 2005 - 19:43 : danielc Can this please be applied to 4-6 and HEAD? ------------------------------------------------------------------------ April 8, 2005 - 06:34 : Dries I was checking settings.php to read up on how to use $db_prefix but noticed that only the 'simple' case was documented. The fact that $db_prefix can be an array isn't explained anywhere (not in the phpdoc of db_prefix_tables() and not in settings.php). Where did that go? I wanted to check what the documentation had to say on the availability of a 'default' entry ... ------------------------------------------------------------------------ April 8, 2005 - 07:11 : moshe weitzman docs in the administrators guide - http://drupal.org/node/2622

1 0

[drupal-devel] [feature] Link to node on comment edit page
by Dries 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/11782 Project: Drupal Version: cvs Component: comment.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: kps Updated by: Dries Status: patch I agree with Steven. This doesn't look like the proper fix. Maybe this is automagically fixed by Moshe's comment patch ... ? Dries Previous comments: ------------------------------------------------------------------------ October 20, 2004 - 17:31 : kps Attachment: http://drupal.org/files/issues/comment.module.patch (579 bytes) We have had users manage to post their comments on the wrong nodes, and when reviewing a newly submitted comment via admin/comment/list/approval, there seems to be no practical way to tell which node the comment belongs to. The attached patch adds a link to the node on the comment edit page. ------------------------------------------------------------------------ March 13, 2005 - 21:06 : killes(a)www.drop.org The patch is in html. ------------------------------------------------------------------------ March 26, 2005 - 20:15 : kps Attachment: http://drupal.org/files/issues/comment-admin-show-node.patch (579 bytes) Interesting! The patch wasn't in HTML when I uploaded it, and the stated size (579 bytes) matches the size of the plain text file, not the size of the HTML. I'll try again.... ------------------------------------------------------------------------ March 27, 2005 - 23:40 : Steven Perhaps this should be fixed through a breadcrumb instead?

1 0

[drupal-devel] [bug] race condition in variable_set in bootstrap.inc
by jhriggs 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/20114 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: Ricetons Reported by: Ricetons Updated by: jhriggs -Status: active +Status: patch Attachment: http://drupal.org/files/issues/variable_set_update.patch (922 bytes) I'm not sure that there is a "right" way to fix this. In mysql, we could use a REPLACE rather than DELETE/INSERT, but that wouldn't work for pgsql. And, if we are really seeing this type of race, there is no telling which REPLACE would actually make it last and be saved. Instead, we could do a check to see if the row exists and do an UPDATE if it does and an INSERT otherwise; however, this would be prone to the same problem if we are seeing such a race. That is, they may both see that the row does not exist and both attempt to do an INSERT. This will only happen the first time a variable is created, though, which may be more acceptable than what we are seeing now. If the row does already exist, they would both do UPDATES and would "succeed" in the sense that there is no duplicate key error. There is still no telling which one actually sticks, though. The attached patch takes the second approach. jhriggs Previous comments: ------------------------------------------------------------------------ April 6, 2005 - 21:53 : Ricetons This bug was exposed by the poormancron module. We can see that the module itself contains no bug. But in the function variable_get(), the function just execute SQL like this: DELETE bar WHERE key="foo" INSERT INTO bar VALUES(foo, bar2ooo) but suppose this: thread A and thread B has executed the DELETE sentence. And they do INSERT at the same time. then an SQL error like this occured: duplicated key 'foo' in table "variables" It's a critical bug. It should be fixed before the time that drupal 4.6.0 released. ------------------------------------------------------------------------ April 8, 2005 - 02:01 : robertDouglass I also experience this on a site that has many different cron cycles ranging from 2 minutes to 6 hours. The query errors which arise (as noted above) are impossible to reproduce through normal testing yet occur fairly (surprisingly) often in the logs. This doesn't just apply to cron jobs but rather to all code that calls variable_set. Setting to CVS since this hasn't been addressed since the issue was opened.

1 0

[drupal-devel] [feature] Separate prefetch request log
by drumm 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19781 Project: Drupal Version: cvs Component: statistics.module Category: feature requests Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch Attachment: http://drupal.org/files/issues/statistics.module.diff (6.15 KB) So Google went and put prefetch in search results [1]. I predict that the semantic HTML with CSS community will start using this more and more. And we might find places to use it in Drupal. Since these prefetch requests aren't exactly the same as other requests we should probably have another log for those hits. [1] http://www.google.com/googleblog/2005/03/enhanced-searching-with-firefox.ht… drumm

2 1

[drupal-devel] [task] Remove hard coded HTML for comment moderation
by drumm 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/20103 Project: Drupal Version: cvs Component: comment.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch Attachment: http://drupal.org/files/issues/comment.module_8.diff (5.34 KB) Removing another place where we have hard coded HTML instead of the form functions. The unpublish selection actually didn't seem to work, so I removed it since we have delete. It should be implemented as a link anyway. drumm

4 5

[drupal-devel] [bug] Make block configuration understandable
by Dries 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/11875 Project: Drupal Version: cvs Component: block.module Category: bug reports Priority: normal Assigned to: killes(a)www.drop.org Reported by: drumm Updated by: Dries Status: patch I'll add this to the release announcement together with the other upgrade issues. Better to centralize everything imho. Dries Previous comments: ------------------------------------------------------------------------ October 21, 2004 - 21:42 : drumm Attachment: http://drupal.org/files/issues/tmp (42.68 KB) The primary goal of this patch is to take the 'custom' and 'path' columns of the block overview page and make them into something understandable. As of Drupal 4.5 'custom' lacked an explanation which wasn't buried in help text and path required dealing with regular expressions. Every block now has a configuration page to control these options. This gives more space to make form controls which do not require a lengthy explanation. This page also gives modules a chance to put their block configuration options in a place that makes sense using new operations in the block hook. The only required changes to modules implementing hook_block() is to be careful about what is returned. Do not return anything if $op is not 'list' or 'view'. Once this change is made, modules will still be compatible with Drupal 4.5. Required changes to core modules are included in this path. An additional optional change to modules is to implement the additional $op options added. 'configure' should return a string containing the configuration form for the block with the appropriate $delta. 'configure save' will come with an additional $edit argument, which will contain the submitted form data for saving. These changes to core modules are also included in this patch. I have posted screenshots at http://foo.delocalizedham.com/tmp/blocks/. Additional work, such as further rearrangement of the block overview page, is as always a task for another patch. ------------------------------------------------------------------------ October 21, 2004 - 23:56 : drumm Attachment: http://drupal.org/files/issues/block_0.diff (42.79 KB) Now with a few more comments. ------------------------------------------------------------------------ October 22, 2004 - 00:14 : drumm Attachment: http://drupal.org/files/issues/blocks.sql (211 bytes) And the DB changes. ------------------------------------------------------------------------ October 23, 2004 - 19:06 : Dries This patch marks a significant usability improvement. Excellent job. I had a quick look at the patch: 1. Taken from the patch: <?php + else if ($op == 'configure' && $delta = 3) { ?> There are a number of '$delta = 3's that probably should be '$delta == 3's. 2. I would use 'save' rather than 'configure save'. 3. block_box_add() is a little weird: 3.a. Is the $bid parameter being used? 3.b. <?php + drupal_set_message(block_box_save($edit)); + drupal_goto('admin/block'); ?> I would move the drupal_set_message() and the drupal_goto() inside block_box_save(), if possible without having to add extra code. 3.c. <?php + $form = block_box_form(); + $form .= form_submit(t('Save block')); + $output .= form($form); ?> Any reason we can't move all form stuff into block_box_form()? 3.d. It would be good if we could get rid of the block vs box thing. 4. We need proper database updates. 5. [?+ php block['pages_show'] = $old_blocks[$module][$delta]->pages_show; + $block['pages'] = $old_blocks[$module][$delta]->pages; ?] What is the difference between 'pages' and 'pages_show'? It is not clear from the names so maybe we should try to come up with more explanatory names. 6. The changes will need to be documented in a new 'Converting modules from 4.5.x to CVS' page. 7. Has the block API Doxygen comments? ------------------------------------------------------------------------ October 23, 2004 - 22:40 : moshe weitzman this is an improvement. one nit: don't use a form_group() around all the form fields on a page. There is no need to group if everything is of the same type. If you need to name that single group of forms, use the page title ------------------------------------------------------------------------ October 23, 2004 - 23:13 : Steven I agree, this is a very nice patch. 1) One thing I do miss is that there is no (longer?) a clear overview of blocks (which blocks are always enabled, which are contextual, ...). However, this is really the fault of the giant configuration table. Reworking that is a different patch. 2) I have a usability issue with block.module which could go in this patch: the description field is really only useful for module-defined blocks where the title is not descriptive. Custom blocks have static titles, so the description is really redundant. I know I never fill it in on my own site, only to curse myself when I see the administration screen and have a bunch of unknown blocks sitting there. So either: - Get rid of the description field for custom blocks and use the title instead. - Keep the description field, but use the block title if the description field was left empty. I prefer the first. 3) Here's a reworked contextual help text for the block admin. The giant paragraph that we have now is not going to be read by anyone: "Blocks are the boxes in the left and right side bars of the web site. They are made available by modules or created manually. Only enabled blocks are shown. You can position the blocks by deciding which side of the page they will show up on (region) and in which order they appear (weight). If you want certain blocks to disable themselves temporarily during high server loads, check the 'Throttle' box. You will need to enable the auto-throttle on the [[throttle configuration page]] after having enabled the throttle module. " 4) The general module help needs work: "If the block\'s throttle checkbox is checked, the throttle level must me low." (me -> be) The paragraph about "A block is visible when:" is also structured oddly. The part about the throttle is confusing and the last option does not match the preceding fragment at all (it starts a new sentence). Maybe it's better to say: " A block's visibility depends on: - Its enabled checkbox. Disabled blocks are never shown. - Its throttle checkbox. Throttled blocks are hidden during high server loads. - Its path options. Blocks can be configured to only show/hide on certain pageS. - User settings. You can choose to let your users decide whether to show/hide certain blocks. - Its function. Dynamic blocks (such as those defined by modules) may be empty on certain pages and will not be shown. " This is a bit vaguer, but shorter and easier to understand. Where the exact options are located is not so important, because they are in a very logical place, and people are bound to run into them when making their own blocks. ------------------------------------------------------------------------ October 24, 2004 - 02:05 : Dries I'm OK with removing the description. ------------------------------------------------------------------------ October 29, 2004 - 02:42 : drumm Attachment: http://drupal.org/files/issues/block_1.diff (44.79 KB) #3: 1. Changed '=' to '=='. 2. Changed 'configure save' to 'save'. 3. Cleaned up block_box_add(), specific code is there because block_box_form() and block_box_save() are reused for editing. The distinction between block and box is useful, boxes are user created blocks, however box might not be the best name. 4. Added database updates for mysql. 5. Pages stores the list of pages for the textarea, pages_show stores whether those are pages to show, or not show, on for the radio buttons. 6. Yes it will. Text can probably be copied from #1. 7. Finishing those up now. I can commit them to contrib cvs myself. #4: It works well on blocks with specific configurations. I expect a limited number of additional options such as block name will be added before 4.6. That will create more groupings. #5: 1. Yes, lets save that for another patch. 2. I have a good idea for the description's needed removal which I plan to work into a patch which allows every block to be renamed. 3. Replaced with the suggested help text. 4. Changed 'me' to 'be'. #6: See #5.2. ------------------------------------------------------------------------ October 29, 2004 - 09:39 : stefan nagtegaal I think - to be more consistent with the rest of drupal - that if the throttle.module is not enabled, you should _not_ display the throttle fields at all. We do this all the time inside drupal, right? I suggest you do something like: <?php - $header = array(t('Block'), t('Enabled'), t('Throttle'), t('Weight'), t('Region'), array('data' => t('Operations'), 'colspan' => 2)); + $header = array(t('Block'), t('Enabled'), (module_exist('throttle') ? t('Throttle')), t('Weight'), t('Region'), array('data' => t('Operations'), 'colspan' => 2)); ?> and: <?php - $rows[] = array($block['info'], array('data' => form_checkbox(NULL, $block['module'] .']['. $block['delta'] .'][status', 1, $block['status']), 'align' => 'center'), array('data' => form_checkbox(NULL, $block['module'] .']['. $block['delta'] .'][throttle', 1, $block['throttle'], NULL, module_exist('throttle') ? NULL : array('disabled' => 'disabled')), 'align' => 'center'), form_weight(NULL, $block['module'] .']['. $block['delta'] .'][weight', $block['weight']), form_radios(NULL, $block['module'] .']['. $block['delta'] .'][region', $block['region'], array(t('left'), t('right'))), l(t('configure'), 'admin/block/configure/'. $block['module'] .'/'. $block['delta']), $operation); + $rows[] = array($block['info'], array('data' => form_checkbox(NULL, $block['module'] .']['. $block['delta'] .'][status', 1, $block['status']), 'align' => 'center'), (module_exist('throttle') ? array('data' => form_checkbox(NULL, $block['module'] .']['. $block['delta'] .'][throttle', 1, $block['throttle'], NULL)), 'align' => 'center'), form_weight(NULL, $block['module'] .']['. $block['delta'] .'][weight', $block['weight']), form_radios(NULL, $block['module'] .']['. $block['delta'] .'][region', $block['region'], array(t('left'), t('right'))), l(t('configure'), 'admin/block/configure/'. $block['module'] .'/'. $block['delta']), $operation); ?> The above patch is untested, and i'm not fully trusted with the fact that it works, but your should get an idea of what I mean.. ------------------------------------------------------------------------ October 29, 2004 - 22:41 : drumm Actually this patch leaves the overview page as is, excpet for removing the columns that are expanded into the configuration page. I believe the standard is to show the throttle configuration, but disabled if not applicable. See the modules administration page. ------------------------------------------------------------------------ October 29, 2004 - 23:15 : Dries I tried the patch: running the update.php script disabled all blocks. Can't look into it right now (lunch) but it is probably an easy fix. user error: Unknown column 'pages_show' in 'field list' query: INSERT INTO blocks (module, delta, status, weight, region, pages_show, pages, custom, throttle) VALUES ('user', '3', 0, 5, 1, 0, '', 0, 1) in /mnt/redhat/foo/cvs/web/drupal.org/cvs/includes/database.mysql.inc on line 125. ------------------------------------------------------------------------ October 30, 2004 - 08:20 : stefan nagtegaal Drumm, I know how the current overview looks and that's why I am asking you to make this fix.. The throttle-checkbox should _not_ be displayed instead of being disabled when the module is not enabled.. If a module is disabled, we don't display their help text and configuration options at all, right? So why are we doing that for the throttle module? Stefan. ------------------------------------------------------------------------ October 31, 2004 - 09:38 : Dries Committed. The UI could do with a tad more polish though, but that can be part of a new patch. Please make sure to update the documentation. ------------------------------------------------------------------------ April 2, 2005 - 20:52 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/update_2.patch (823 bytes) The upgrade simply dropped all the stored regexes. I understand that an upgrade patch was difficult to provide, but we should at least document this. Attached patch does this.

1 0

[drupal-devel] [bug] Cannot delete users
by Dries 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19750 Project: Drupal Version: cvs Component: user.module Category: bug reports Priority: critical Assigned to: Anonymous Reported by: AtomicMaestro Updated by: Dries Status: patch Could this be fixed properly by making a stand-alone callback for the delete view? Dries Previous comments: ------------------------------------------------------------------------ March 31, 2005 - 11:33 : AtomicMaestro I can't seem to delete users. To reproduce: 1. Create a user. 2. View the user created in 1. 3. Click Delete I would expect to be told I have to block the user first, but instead, I'm sent back to the user report. There's no message, nothing in the log. ------------------------------------------------------------------------ April 1, 2005 - 09:40 : asimmonds Attachment: http://drupal.org/files/issues/fix_delete_user.patch (581 bytes) Patch attached to fix this, the delete code in node.module works the same way ------------------------------------------------------------------------ April 3, 2005 - 16:15 : moshe weitzman when one deletes a user from list of users in admin, he expects to return back to that list after successfully deleting. thats the intent of the $_REQUEST['destination']. I think this patch will defeat that behavior. There certainly is a bug here, but I don't think we have found the right fix yet. I suggest testing a delete from both admin and user/edit pages. make sure you arrive at the expected page after a successful delete. ------------------------------------------------------------------------ April 4, 2005 - 09:19 : asimmonds There are two methods to enter the user edit page as a admin: 1/ View a user, then select the edit tab. 2/ Click 'edit' for the user in the admin/user list. The 2nd method sets destination=admin/user in the URL, when the user edit page is shown. If the Delete button is clicked, it calls drupal_goto("user/$account->uid/delete") (the code's comment mentions this is done to make the tabs disappear). As destination is already set, drupal_goto() redirects to that ie. "admin/user" instead of "user/$account->uid/delete" If after successfully deleting a user, the code calls drupal_goto("admin/user"). I looked at how node.module does the node delete/confirm sequence and it has this unset($_REQUEST['destination']) in there as well. Maybe this delete/confirm sequence pattern needs a re-think for 4.7 ------------------------------------------------------------------------ April 6, 2005 - 06:01 : moshe weitzman Attachment: http://drupal.org/files/issues/user_15.patch (1020 bytes) here is an alternative. this one properly maintains the destination parameter which is useful especially when you are not on the 1st page of your user listing. the redirect insures that you return to the page you were on. another advantage is that i was able to delete a few lines of code which were performing a drupal_goto(). the downside is that the pupose of the drupal_goto() was to get rid of the tabs. I don't mind the tabs on the delete confirm page, so I think my patch is better, and removes a couple lines of code. either patch presented here would work ok. ------------------------------------------------------------------------ April 6, 2005 - 07:13 : moshe weitzman Attachment: http://drupal.org/files/issues/menu_set_location.patch (2.23 KB) here is a different patch suggested by UnConed. Uses menu_set_location() to remove the tabs on the delete confirmation page for both nodes and users. ------------------------------------------------------------------------ April 6, 2005 - 21:28 : Dries Mmm, Steven's patch fixes the tabs but still is a bit of a hack. (Also, parts of the strings can't be translated, the use of arg(1) vs $edit['nid'] looks a bit weird.) Can't we fix this properly?

1 0

[drupal-devel] [bug] have db_prefix_tables() handle 'default' key not being set
by moshe weitzman 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19609 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: danielc Updated by: moshe weitzman Status: patch docs in the administrators guide - http://drupal.org/node/2622 moshe weitzman Previous comments: ------------------------------------------------------------------------ March 28, 2005 - 16:24 : danielc Attachment: http://drupal.org/files/issues/database.diff (720 bytes) The db_prefix_tables() function in includes/database.inc generates notices if $db_prefix['default'] is not set. This patch fixes that problem. This has a side benefit of making $db_prefix easier to use. ------------------------------------------------------------------------ March 28, 2005 - 17:06 : moshe weitzman nice ... this has been a bit a pimple for a while. ------------------------------------------------------------------------ March 29, 2005 - 10:40 : danielc Attachment: http://drupal.org/files/issues/database2.diff (1.17 KB) I've come up with a new version of the patch that optimizes the speed of the whole function. ------------------------------------------------------------------------ March 29, 2005 - 10:56 : danielc Change Version from "4.5.2" to "cvs". ------------------------------------------------------------------------ March 29, 2005 - 11:01 : danielc Pardon me, I'm testing if this goes the drupal-dev list now that the version is "cvs." ------------------------------------------------------------------------ April 3, 2005 - 07:45 : Dries Any idea of how much faster the second patch is? I don't think it makes a real difference in which case I'd favor the first patch. ------------------------------------------------------------------------ April 3, 2005 - 13:40 : danielc The speed increase is 5%. ------------------------------------------------------------------------ April 3, 2005 - 13:52 : Dries 5% compared to what? What did you measure? ------------------------------------------------------------------------ April 3, 2005 - 23:56 : danielc I tested the db_prefix_tables() function in two configurations. With my first patch applied and with my second patch applied. The second patch is 5% faster than the first. This improvement is the same for both the string and array formats of $db_url. ------------------------------------------------------------------------ April 7, 2005 - 19:43 : danielc Can this please be applied to 4-6 and HEAD? ------------------------------------------------------------------------ April 8, 2005 - 06:34 : Dries I was checking settings.php to read up on how to use $db_prefix but noticed that only the 'simple' case was documented. The fact that $db_prefix can be an array isn't explained anywhere (not in the phpdoc of db_prefix_tables() and not in settings.php). Where did that go? I wanted to check what the documentation had to say on the availability of a 'default' entry ...

1 0