- Development - lists.drupal.org

[drupal-devel] [bug] Can't delete subjects with '&' in title
by Souvent22 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/27140 Post a follow up: http://drupal.org/project/comments/add/27140 Project: Drupal Version: cvs Component: contact.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: Souvent22 Status: patch (code needs review) Hello. Thanks for the feedback. I'm in the middle of fixing it, but i have a question. You'll have to excuse my ignorance with PostGres, I'm not as familiar with it as I am MySQL. With that said, I have a question..... I am trying to make my update where one does not lose data. However, in PostGres, I have to make a seperate "temp" table before hand and then copy the date over the the new table (cause i don't know the exact name of the constraint to drop e.g. the primary key). Basically I'm asking if there is a better suggestion in which I doint have to use a temp table, and i could just manipulate the original table? Thanks. Souvent22 Previous comments: ------------------------------------------------------------------------ Wed, 20 Jul 2005 14:57:03 +0000 : m3avrck Using the site-wide contact forms, any subjects with an '&' cannot be deleted. Clicking delete for these subjects brings up the expected action flow, however, confirming the delete does nothing. Subject still exists. ------------------------------------------------------------------------ Mon, 25 Jul 2005 13:45:38 +0000 : m3avrck It appears this issue might be linked with this one: http://drupal.org/node/23685 A problem with mod_rewrite and not the actual contact module. ------------------------------------------------------------------------ Wed, 31 Aug 2005 16:51:56 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact.module_6.patch (4.7 KB) Redid how contacts work. It now references contacts by ID instead of their category as the unique identifier. It also allows for contacts to have weights, so that one may control where the contact will appear in the listing. ------------------------------------------------------------------------ Wed, 31 Aug 2005 16:53:01 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/updates.inc (27.23 KB) This is the update.inc for the database cahnges that goes along with this patch. ------------------------------------------------------------------------ Wed, 31 Aug 2005 18:31:37 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact.module_7.patch (4.5 KB) Updated patch. ------------------------------------------------------------------------ Wed, 31 Aug 2005 18:31:55 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/updates.inc_4.patch (1.29 KB) Updated database patch. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:17:21 +0000 : Cvbge Hi, 0. You should keep all changes in one file, i.e. pake one patch, not patch for every file 1. You should include changes for database/database.{mysql,pgsql} 2. IIRC auto_increment was depreciated, you should use db_next_id() (but I see auto_increment in cvs...) 3. You use "WHERE cid = '%s'", but cid is integer so this should probably be %d (although this probably would not break anything..) ------------------------------------------------------------------------ Wed, 07 Sep 2005 17:31:21 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact_full.patch (6.38 KB) Ok, I've taken your suggestions in, and i think I have it this time. Let me know if this is a correct format for a patch. This only fixes the bug of not being able to delete special characters. ------------------------------------------------------------------------ Wed, 07 Sep 2005 17:37:30 +0000 : m3avrck Check your database updates, the UNIQUE fields don't look right syntactically. ------------------------------------------------------------------------ Wed, 07 Sep 2005 18:23:07 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact_full_0.patch (6.25 KB) Ok, i apologize for the last upload. Pleasle ignore. I send the wrong file. :-[. Anyway, here's a tested, tried, and correct patch. ------------------------------------------------------------------------ Wed, 07 Sep 2005 18:26:33 +0000 : m3avrck +1 patch works great! Latest HEAD version applied with update.php works great, modifies database correctly for MySQL (and PostgreSQL looks correct as well) and contacts can now be deleted if they have '&' in the subject. Also, dumped entire database and created from modified database.mysql and everything works great. I'd say this patch is ready to be committed! ------------------------------------------------------------------------ Wed, 07 Sep 2005 21:44:47 +0000 : Cvbge Hi, patch does not applies to currect cvs anymore. For the database.pgsql you need to remove (11) from int(11) and also first "category" from UNIQUE line, so it looks like this: CREATE TABLE contact ( cid int NOT NULL, category varchar(255) NOT NULL default '', recipients text NOT NULL default '', reply text NOT NULL default '', UNIQUE (category), PRIMARY KEY (cid) ); The updates are also not compatibile with postgres, but postgres upgrade path is completly broken in cvs and I'll fix it later, so do not pay attention to it.

1 0

[drupal-devel] [bug] Taxonomy_save_* return values
by chx 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/29102 Post a follow up: http://drupal.org/project/comments/add/29102 Project: Drupal Version: cvs Component: taxonomy.module Category: bug reports Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/taxonomy_save_return.patch (4.08 KB) What is described on http://drupal.org/node/22218 is nice. What is in the core is super ugly. I surely was not paying attention when did slipped in. chx

6 9

[drupal-devel] [bug] Blocks showing up when module disabled
by drumm 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/13802 Post a follow up: http://drupal.org/project/comments/add/13802 Project: Drupal Version: cvs Component: module system Category: bug reports Priority: minor Assigned to: drumm Reported by: Jose A Reyero Updated by: drumm Status: patch (code needs work) Yes, I am not going to work on my approach currently. The patch was broken by the recent addition of multiple block regions. I may have time in the future. I recomend making a real patch file for the simpler approach as describe in update #2. drumm Previous comments: ------------------------------------------------------------------------ Thu, 02 Dec 2004 13:19:31 +0000 : Jose A Reyero This only happens in the module administration page. To reproduce this behaviour: 1. enable a module and one block provided by it - I.e. 'archive' module 2. disable the module The block still shows up in the module administration page I find this more funny than annoying and it happens only in module admin page. Anyway, think it's and undesirable behaviour and should be addressed, maybe at the block management level. ------------------------------------------------------------------------ Thu, 16 Jun 2005 04:37:45 +0000 : deekayen I verified this in HEAD for the archive module. ------------------------------------------------------------------------ Thu, 11 Aug 2005 20:49:44 +0000 : tostinni This still apply to HEAD, the procedure to reproduce the bug is the right one. However, I don't understand exactly why the block keep appears on this page, and not on the others pages. The problem is that looking at the blocks table, "archive" block stay there even if it doesn't appear on other pages (on module page yes it does). When I enter a new page, saying home, archive block isn't there but is still on the db. The cache is flushed, because archive seems not to appear on the entry giving me the menu, but the current page keep the old cache entry (well I guess). Ok so I proposed a radical way to correct this : rehashing blocks after saving the module configuration. So I just add a _block_rehash(); after the menu_rebuild();, it's a little dirty but do the trick. If you have any suggestion about a better way to do this ;) <?php function system_listing_save($edit = array()) { $op = $_POST['op']; $edit = $_POST['edit']; if ($op == t('Save configuration')) { db_query("UPDATE {system} SET status = 0 WHERE type = '%s'", $edit['type']); foreach ($edit['status'] as $name => $status) { // Make certain that the default theme is enabled to avoid user error if (($edit['type'] == 'theme') && ($edit['theme_default'] == $name)) { $status = 1; } db_query("UPDATE {system} SET status = %d, throttle = %d WHERE type = '%s' AND name = '%s'", $status, $edit['throttle'][$name], $edit['type'], $name); } if ($edit['type'] == 'theme') { variable_set('theme_default', $edit['theme_default']); } menu_rebuild(); _block_rehash(); drupal_set_message(t('The configuration options have been saved.')); drupal_goto($_GET['q']); } } ?> ------------------------------------------------------------------------ Thu, 11 Aug 2005 22:17:26 +0000 : drumm I can work on this. ------------------------------------------------------------------------ Fri, 12 Aug 2005 00:20:14 +0000 : drumm Attachment: http://drupal.org/files/issues/system.module_5.diff (3.54 KB) I decided to implement a hook system for this. As a side effect: - system_listing_save() only does database things. The UI things such as drupal_set_message() are now separated into the calling function. - The status message for saving module is now: * The aggregator module has been disabled. * The archive module has been enabled. instead of "The configuration options have been saved." - The theme-specific things in system_listing_save() are not there anymore. - There is also a hook called when modules are enabled. I think I'll go write some documentation for the two new hooks now. ------------------------------------------------------------------------ Fri, 12 Aug 2005 00:48:40 +0000 : drumm Here is some hook documentation: <?php /** * This hook is called on every module for when each new module is enabled. The * module will be loaded at this point so the module's own hook will be called * and other modules may use module_invoke() to call functions in the module. * * @param $module * The name of the module which has just been enabled. */ hook_module_enabled($module) { } /** * This hook is called on every module for when each new module is disabled. * The module will still be loaded at this point so the module's own hook will * be called and other modules may use module_invoke() to call functions in the * module. * * @param $module * The name of the module which has just been disabled. T */ hook_module_disabled($module) { } ?> ------------------------------------------------------------------------ Wed, 07 Sep 2005 19:18:22 +0000 : drumm This patch is now fubar. Please feel free to resubmit the concept in update #2 by tostinni in patch form. ------------------------------------------------------------------------ Wed, 07 Sep 2005 20:17:08 +0000 : tostinni Sorry I didn't understood "This patch is now fubar." ? This patch has been abandonned ?

1 0

[drupal-devel] [feature] Flat out $node in phptemplate_node vars
by chx 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/24862 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/nodevarsflat.patch (907 bytes) I'd like to write $whatever in node.tpl.php instead of $node->whatever . Hence the patch. chx

6 9

[drupal-devel] stepping through the revision history with AJAX
by Moshe Weitzman 08 Sep '05

08 Sep '05

there are 2 fantastic greasemonkey scripts out there which use AJAX to view the revision history of a wikipedia page. It would be great if we offerred this as a Contrib module, or even in core. Our AJAX and revision APIs have all the plumbing already. If you are busy, just watch this Flash movie - . Otherwise, see http://www.waxy.org/archive/2005/06/27/wikipedi.shtml If you are new to Greasemonkey, see http://www.google.com/url?sa=t&ct=res&cd=1&url=http%3A//greasemonkey.mozdev…

2 1

[drupal-devel] [bug] fix empty form elements
by tostinni 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/18663 Post a follow up: http://drupal.org/project/comments/add/18663 Project: Drupal -Version: 4.6.0 +Version: 4.6.3 Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: tostinni -Status: active +Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/fix_form-4.6.3.patch (3.54 KB) "It's strange that this problem still appears in drupal 4.6.3 (August 15, 2005); 4.6 CVS, and CVS Head and has been marked fixed on May 21, 2005. The last patch (fix_form_3.patch from comment 20) still applies with little fuzziness and fixes the problem. " The problem has been fixed in HEAD as Dries said in #21 HEAD refer to the future 4.7 version. Here is the updated patch that apply to 4.6.3 tostinni Previous comments: ------------------------------------------------------------------------ Thu, 10 Mar 2005 01:51:56 +0000 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ Sat, 12 Mar 2005 18:47:42 +0000 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ Mon, 28 Mar 2005 16:27:03 +0000 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ Mon, 28 Mar 2005 16:32:16 +0000 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ Mon, 28 Mar 2005 17:36:01 +0000 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field... ------------------------------------------------------------------------ Fri, 08 Apr 2005 07:19:28 +0000 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ Mon, 11 Apr 2005 23:15:05 +0000 : chx Dries? ------------------------------------------------------------------------ Tue, 12 Apr 2005 19:15:52 +0000 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ Tue, 12 Apr 2005 19:39:00 +0000 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ Tue, 12 Apr 2005 21:23:06 +0000 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad. ------------------------------------------------------------------------ Tue, 12 Apr 2005 22:51:25 +0000 : jhriggs -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select. ------------------------------------------------------------------------ Sat, 23 Apr 2005 11:35:30 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form.patch (4.71 KB) See Gordon's complaint http://lists.drupal.org/archives/drupal-devel/2005-04/msg00934.html ------------------------------------------------------------------------ Sat, 23 Apr 2005 11:38:07 +0000 : chx Forgot to change the title. ------------------------------------------------------------------------ Sat, 23 Apr 2005 11:46:46 +0000 : chx It seems that it's not obvious why this is necessary. The calling code can not always handle itself the situation if the save is done by system_save_settings. The edge case is: I set up something, so variable_get no longer falls to its default value, later I decide to empty it. If we do not do something like this patch, system_save_settings can not know there is something to be set to 0/array() because it is just not in $_POST. ------------------------------------------------------------------------ Sat, 23 Apr 2005 13:23:37 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form_0.patch (4.72 KB) Small bugfix. ------------------------------------------------------------------------ Sat, 23 Apr 2005 14:53:05 +0000 : chx doh. still has serious problems when the name is an array. stay tuned. ------------------------------------------------------------------------ Sun, 24 Apr 2005 09:13:00 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form_1.patch (4.97 KB) Recursive version. ------------------------------------------------------------------------ Sun, 24 Apr 2005 12:51:18 +0000 : Dries Patch looks good but I would add some phpdoc to document what and why we are doing this. ------------------------------------------------------------------------ Sat, 21 May 2005 16:43:47 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form_2.patch (3.23 KB) People told me that form_radios does not need this because we can reasonably expect that a radio is checked, that's different between a bunch of checkboxes and a bunch of radios. form_select submits an empty array (as also told by some). So we are back to checkbox and checkboxes which need the fix. ------------------------------------------------------------------------ Sat, 21 May 2005 16:55:24 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form_3.patch (3.24 KB) ------------------------------------------------------------------------ Sat, 21 May 2005 18:34:20 +0000 : Dries Committed to HEAD. Thanks. ------------------------------------------------------------------------ Mon, 23 May 2005 00:09:30 +0000 : TDobes Why do we now have both a _fix_checkboxes() and fix_checkboxes() function? Shouldn't we name these something a little more intuitive? Without poking through the code, I have no idea what _fix_checkboxes is for and why it's different than fix_checkboxes. It really deserves some phpdoc too. Any sort of more-pleasant variable naming in _fix_checkboxes() would be much appreciated as well... It's easy to get lost in that array1/array2/single letter variable business pretty fast. Sorry for nit-picking, but most of the Drupal core code is far more readable than this. ------------------------------------------------------------------------ Tue, 06 Sep 2005 03:00:14 +0000 : mr700 It's strange that this problem still appears in drupal 4.6.3 (August 15, 2005); 4.6 CVS, and CVS Head and has been marked fixed on May 21, 2005. The last patch (fix_form_3.patch from comment 20 [1]) still applies with little fuzziness and fixes the problem. [1] http://drupal.org/node/18663#comment-30411

1 0

[drupal-devel] callback arguments
by Gerhard Killesreiter 08 Sep '05

08 Sep '05

Hi there, today Dries has a committed a patch that removes the last occurrence of a non-array menu callback argument from core. It is actually documented that those should be arrays, but in php4 it also works with strings. The following contrib modules need fixing: banner/banner.module: cck/content.module: contact_dir/contact_dir.module: diff/diff.module: htmlarea/htmlarea.module: libdb/libdb.module: paypal_tipjar/paypal_tipjar.module: pdfview/pdfview.module: periodical/periodical.module: quiz/quiz.module: role_to_file/role_to_file.module: stats_plot/stats_plot.module: trackback/trackback.module: video/video.module: weblink/weblink.module: Cheers, Gerhard

1 0

[drupal-devel] [bug] Block table: no key/index defined
by Souvent22 07 Sep '05

07 Sep '05

Issue status update for http://drupal.org/node/27157 Post a follow up: http://drupal.org/project/comments/add/27157 Project: Drupal Version: cvs Component: database system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: Souvent22 Status: patch (code needs review) I agree taht some sort of indexing needs to be done on the blocks. Although, on "most" sites, this is not a problem. But, with any scale (be it many many blocks, or many many hits), the blocks will be retrived either often (site with many hits) or become a large query (many blocks, however if you have that many blocks, you may want to look at your site design). Either way, this would make for a performance improvement. I know when optimizing a site, especially one that recieves many hits; it's hard to find just one improvement, it's usually the little tweaks hear and there that result in an overall large perfomance gain. Souvent22 Previous comments: ------------------------------------------------------------------------ Wed, 20 Jul 2005 18:34:30 +0000 : m3avrck The block table currently has no primary key or index defined. This greatly hurts queries on the table, as the SQL server has no way to introduce fast, index, based searching, which can be cachable. More info: http://www.odetocode.com/Articles/237.aspx I recommend the following change: PRIMARY KEY(module,delta); That should satisfy the key requirements for the table and speed up queries on this table, with most noticeable results on websites that rely on lots of modules. Fix should be backwards compatible as well. I'd post a patch for this database change but I'm not sure the exact procedures for posting a database change patch, not as straightforward, so if anyone cares, that would be great. Nice little, almost free performance benefit for the next HEAD version :-) ------------------------------------------------------------------------ Sat, 23 Jul 2005 10:28:56 +0000 : djnz Disagree. Blocks are infrequently retrieved from the table by module and delta. In fact the only query that is run on every page is SELECT * FROM {blocks} WHERE status = 1 AND region IN ('%s') ORDER BY weight, module ... so status and region would be the first things to index (unless you have a LOT of blocks active, weight and module are not going to make any significant impact). ------------------------------------------------------------------------ Mon, 25 Jul 2005 02:08:29 +0000 : m3avrck Ok I do agree with you and maybe my post wasn't super optimized. There should be some sort of *unique* key on the table and drumm agreed with me in #drupal that PRIMARY KEY(module,delta) would be a great candidate. As for the indexes, we can index the status and region since those are fields that queries actually act upon. But we should have definetly some sort of unique key defined as well for the best optimization, along with an index on most queried fields. ------------------------------------------------------------------------ Wed, 07 Sep 2005 14:48:56 +0000 : m3avrck Going to bump this, I think we can get this performance fix into 4.7. I would suggest based on comments: PRIMARY KEY(module,delta);<br> INDEX(status,region); If anything, the PRIMARY KEY could be left off, however I see this as a small additional performance boost... only hinderance on performace would be on insert new blocks... which is significantly less frequent than actually showing/displaying blocks. ------------------------------------------------------------------------ Wed, 07 Sep 2005 15:23:15 +0000 : m3avrck Attachment: http://drupal.org/files/issues/database_2.patch (2.46 KB) Here's a patch that adds this support to the latest HEAD. Tested on MySQL, both the update patch works and dropping/recreating database patch works. Comments welcome on logistics of key/index, but otherwise should be a nice little performance boost for 4.7. ------------------------------------------------------------------------ Wed, 07 Sep 2005 21:51:13 +0000 : m3avrck Will reroll patch tomorrow (seeing lots of updates going in right now).

1 0

[drupal-devel] [bug] Block table: no key/index defined
by m3avrck 07 Sep '05

07 Sep '05

Issue status update for http://drupal.org/node/27157 Post a follow up: http://drupal.org/project/comments/add/27157 Project: Drupal Version: cvs Component: database system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck Status: patch (code needs review) Will reroll patch tomorrow (seeing lots of updates going in right now). m3avrck Previous comments: ------------------------------------------------------------------------ Wed, 20 Jul 2005 18:34:30 +0000 : m3avrck The block table currently has no primary key or index defined. This greatly hurts queries on the table, as the SQL server has no way to introduce fast, index, based searching, which can be cachable. More info: http://www.odetocode.com/Articles/237.aspx I recommend the following change: PRIMARY KEY(module,delta); That should satisfy the key requirements for the table and speed up queries on this table, with most noticeable results on websites that rely on lots of modules. Fix should be backwards compatible as well. I'd post a patch for this database change but I'm not sure the exact procedures for posting a database change patch, not as straightforward, so if anyone cares, that would be great. Nice little, almost free performance benefit for the next HEAD version :-) ------------------------------------------------------------------------ Sat, 23 Jul 2005 10:28:56 +0000 : djnz Disagree. Blocks are infrequently retrieved from the table by module and delta. In fact the only query that is run on every page is SELECT * FROM {blocks} WHERE status = 1 AND region IN ('%s') ORDER BY weight, module ... so status and region would be the first things to index (unless you have a LOT of blocks active, weight and module are not going to make any significant impact). ------------------------------------------------------------------------ Mon, 25 Jul 2005 02:08:29 +0000 : m3avrck Ok I do agree with you and maybe my post wasn't super optimized. There should be some sort of *unique* key on the table and drumm agreed with me in #drupal that PRIMARY KEY(module,delta) would be a great candidate. As for the indexes, we can index the status and region since those are fields that queries actually act upon. But we should have definetly some sort of unique key defined as well for the best optimization, along with an index on most queried fields. ------------------------------------------------------------------------ Wed, 07 Sep 2005 14:48:56 +0000 : m3avrck Going to bump this, I think we can get this performance fix into 4.7. I would suggest based on comments: PRIMARY KEY(module,delta);<br> INDEX(status,region); If anything, the PRIMARY KEY could be left off, however I see this as a small additional performance boost... only hinderance on performace would be on insert new blocks... which is significantly less frequent than actually showing/displaying blocks. ------------------------------------------------------------------------ Wed, 07 Sep 2005 15:23:15 +0000 : m3avrck Attachment: http://drupal.org/files/issues/database_2.patch (2.46 KB) Here's a patch that adds this support to the latest HEAD. Tested on MySQL, both the update patch works and dropping/recreating database patch works. Comments welcome on logistics of key/index, but otherwise should be a nice little performance boost for 4.7.

1 0

[drupal-devel] [bug] Can't delete subjects with '&' in title
by Cvbge 07 Sep '05

07 Sep '05

Issue status update for http://drupal.org/node/27140 Post a follow up: http://drupal.org/project/comments/add/27140 Project: Drupal Version: cvs Component: contact.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: Cvbge Status: patch (code needs review) Hi, patch does not applies to currect cvs anymore. For the database.pgsql you need to remove (11) from int(11) and also first "category" from UNIQUE line, so it looks like this: CREATE TABLE contact ( cid int NOT NULL, category varchar(255) NOT NULL default '', recipients text NOT NULL default '', reply text NOT NULL default '', UNIQUE (category), PRIMARY KEY (cid) ); The updates are also not compatibile with postgres, but postgres upgrade path is completly broken in cvs and I'll fix it later, so do not pay attention to it. Cvbge Previous comments: ------------------------------------------------------------------------ Wed, 20 Jul 2005 14:57:03 +0000 : m3avrck Using the site-wide contact forms, any subjects with an '&' cannot be deleted. Clicking delete for these subjects brings up the expected action flow, however, confirming the delete does nothing. Subject still exists. ------------------------------------------------------------------------ Mon, 25 Jul 2005 13:45:38 +0000 : m3avrck It appears this issue might be linked with this one: http://drupal.org/node/23685 A problem with mod_rewrite and not the actual contact module. ------------------------------------------------------------------------ Wed, 31 Aug 2005 16:51:56 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact.module_6.patch (4.7 KB) Redid how contacts work. It now references contacts by ID instead of their category as the unique identifier. It also allows for contacts to have weights, so that one may control where the contact will appear in the listing. ------------------------------------------------------------------------ Wed, 31 Aug 2005 16:53:01 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/updates.inc (27.23 KB) This is the update.inc for the database cahnges that goes along with this patch. ------------------------------------------------------------------------ Wed, 31 Aug 2005 18:31:37 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact.module_7.patch (4.5 KB) Updated patch. ------------------------------------------------------------------------ Wed, 31 Aug 2005 18:31:55 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/updates.inc_4.patch (1.29 KB) Updated database patch. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:17:21 +0000 : Cvbge Hi, 0. You should keep all changes in one file, i.e. pake one patch, not patch for every file 1. You should include changes for database/database.{mysql,pgsql} 2. IIRC auto_increment was depreciated, you should use db_next_id() (but I see auto_increment in cvs...) 3. You use "WHERE cid = '%s'", but cid is integer so this should probably be %d (although this probably would not break anything..) ------------------------------------------------------------------------ Wed, 07 Sep 2005 17:31:21 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact_full.patch (6.38 KB) Ok, I've taken your suggestions in, and i think I have it this time. Let me know if this is a correct format for a patch. This only fixes the bug of not being able to delete special characters. ------------------------------------------------------------------------ Wed, 07 Sep 2005 17:37:30 +0000 : m3avrck Check your database updates, the UNIQUE fields don't look right syntactically. ------------------------------------------------------------------------ Wed, 07 Sep 2005 18:23:07 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact_full_0.patch (6.25 KB) Ok, i apologize for the last upload. Pleasle ignore. I send the wrong file. :-[. Anyway, here's a tested, tried, and correct patch. ------------------------------------------------------------------------ Wed, 07 Sep 2005 18:26:33 +0000 : m3avrck +1 patch works great! Latest HEAD version applied with update.php works great, modifies database correctly for MySQL (and PostgreSQL looks correct as well) and contacts can now be deleted if they have '&' in the subject. Also, dumped entire database and created from modified database.mysql and everything works great. I'd say this patch is ready to be committed!

1 0