Category Column too small for Gallery searches

wmswms - July 4, 2006 - 16:24
Project:Zeitgeist
Version:4.7.x-1.x-dev
Component:Miscellaneous
Category:bug report
Priority:minor
Assigned:fgm
Status:closed
Description

When you use the Gallery2 module, its search category is "gallery". The Category column in the zeitgeist table is VARCHAR(5). So what gets stored is "galle", not "gallery". This breaks things like "latest 5 searches". I didnt bother with a .patch file, but the line in the two mysql files that read:

`category` varchar(5) NOT NULL default 'node',

Should read:

`category` varchar(20) NOT NULL default 'node',

Or change 20 to an even bigger number as needed. The SQL to alter a table already installed is:

ALTER TABLE zeitgeist CHANGE category category VARCHAR(20) DEFAULT 'node' NOT NULL

#1

fgm - July 4, 2006 - 16:53

Interesting.

To make it more general, could you check what the maximum length is for such a category, taking into account the fields holding them throughout the drupal data model ? That way we can be sure we'll have the exact minimum (hence most efficient) length able to hold any category.

#2

wmswms - July 4, 2006 - 18:10

Im new to Drupal, but it seems that the categories can be defined by any module to be any length? It might be best just to use VARCHAR(255) to allow the most room. Since it is a VAR field, it wouldn't really waste any room as the field is only as big as what its holding, correct?

#3

fgm - July 4, 2006 - 19:41

I did some searching, because I dislike using uncontrolled values. Here is what I found.

These category name are actually the names of the modules implementing hook_search. As of today, the list of modules actually implementing this hook in 4.7.x is surprisingly short. Core modules are emphasized.

  • node
  • user
  • civinode
  • gallery
  • package jobsearch: job and resume
  • package playlist: audio_playlist

So, all in all, in existing drupal core and contrib modules, the longest module name that can appear in a search appears to be audio_playlist, 14 characters long. This is also the max length of a file name on old UNIX filesystems, BTW, but there are not considered to meet drupal prerequisites, so we can't take that into account.

Another criterium is the shortest length defined for a module name in drupal core. This is 64 chars in table blocks ; other tables in core allow module names up to 128 and 255 chars respectively, but blocks mandates that shorter limit. So we can take that to be the official limit until core evolves.

So column zeitgeist/category will now be a varchar(64), to follow the rule set by column blocks/module

#4

fgm - July 4, 2006 - 20:03
Assigned to:Anonymous» fgm
Status:active» needs review

You can download and test the latest version from CVS.

You can use update.php or devel.module to force the DB update if you didn't already do it by hand.

Please check if all works well and close the issue if your problem is solved.

#5

wmswms - July 4, 2006 - 21:23

I noticed you didnt change the .mysql* files but just added in an update() hook? Well I wrote up an install() for you based on the .mysql files, but using the bigger column size. I believe its possible to use PHP 4.1 but still use the "mysql" db extension, so Im not sure if this will cause a problem. For now I used the 4.0 syntax for "mysql" and 4.1 syntax for "mysqli".

AttachmentSize
zeitgeist.install 1.36 KB

#6

fgm - July 5, 2006 - 12:56
Status:needs review» needs work

Thanks for trying. Some improvements still need to be done, though:

  • use curly braces {} around table names for prefix independence
  • AIUI the mysql/mysqli methods are not dependent on 4.1 vs 4.0, but on the server config. This may change the tests you are performing
  • please use the same coding style as the rest of the module and the g2 module (this is not exactly the same set of rules as for drupal core)
  • as it appears you are obviating the need for the *.mysql* files, this needs matching changes in the documentation files: INSTALL.txt, CHANGELOG.txt, that will need patching too.

#7

fgm - July 8, 2006 - 17:59

Gman submitted another version of the zeitgeist.install file on http://drupal.org/node/72662

It would be nice to merge the two versions so we can have just one complete .install.

#8

fgm - January 2, 2007 - 19:54
Priority:normal» minor
Status:needs work» fixed

The "category" width part of the issue has been fixed since 1.9.2.1 in June, so I'm closing the issue. Opening a separate issue for the "install" part.

#9

Anonymous - January 16, 2007 - 20:01
Status:fixed» closed
 
 

Drupal is a registered trademark of Dries Buytaert.