Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

There is a similar location-row in the watchdog table.

The MySQL database scheme also uses varchar(128) so it is not specific to PostgreSQL.

I wouldn't recommend making it limitless. Maybe 255 is more sensible?

patryk’s picture

Title: Field location in table locales_source is too short in PostgreSQL » Field location in table locales_source is too short
Component: postgresql database » database system
FileSize
686 bytes

Hmm... maybe you're right. I came accross a polish language pack which had 177 characters in that field, it could easily have more, but I think 256 will be good for now.

The field in watchdog is not revelant to this bug as it handles the filename of event occured and can contain only one location, unlike many as here.

The new patch attached and bug details changed.

Dries’s picture

Committed to HEAD and DRUPAL-4-6. Thanks patryk.

jhriggs’s picture

The field was changed to 256 by this patch. Not sure about pgsql, but the max for a CHAR/VARCHAR in mysql is 255. Patch attached.

Note: Shouldn't there also be a update in updates.inc for this since it is a schema change?

jhriggs’s picture

Version: » 4.6.0
Priority: Normal » Critical

I didn't realize the patch had been applied to 4.6 also. Marking as critical.

patryk’s picture

FileSize
1.83 KB

I made a small investigation. Postgres eats 1 GB strings for breakfast unlike MySQL which sucks with it's 255 character limit until 5.0.3 excluding ;) (even 5.0.3 has limit of 65536 chars)

Some references:
http://www.postgresql.org/docs/7.4/interactive/datatype-character.html - as Drupal must use v7.4 at least
http://dev.mysql.com/doc/mysql/en/char.html

In the result let's do that field 255 chars long in both databases to be coherent.

PS: I'm thinking about something to ease pgsql 7.4's column type change. 8.0 has it already implemented. Maybe some hyper-clever function? ;)

The new patch against today's CVS attached. Includes updates in updates.inc.

drumm’s picture

+1 for the database.mysql part which I sucessfully used. That file in the 4.6 branch is rather broken.

Dries’s picture

Committed to HEAD. The update.inc-part did not apply against HEAD though. Marking this active.

baudolino’s picture

FileSize
1.38 KB

This should patch update.inc

patryk’s picture

FileSize
1.21 KB

baudolino has provided the patch for HEAD's update.inc, I'm providing the same for DRUPAL-4-6's. Hope it is the last patch for this task :)

Dries’s picture

Patches no longer apply. Sorry. If time permits, please update the patches.

baudolino’s picture

Here's the patch for HEAD.

patryk’s picture

patch against DRUPAL-4-6. Commit fast ;)

Dries’s picture

Committed to DRUPAL-4-6. Thanks for your persistence Patryck. Marking this 'active' until we have an updated patch for HEAD.

baudolino’s picture

The patch I submitted for HEAD before patryk sent his for 4.6 is updated and applies! Don't let the 135 in the name fool you, I just inserted it before the last update (which becomes 136), since chronologically that's what happened. Why would it make a difference in HEAD the order in which they're numbered?

baudolino’s picture

Nevermind, didn't notice the silent update. Here we go again. Update 137 for HEAD coming up!

Dries’s picture

Committed to HEAD. Thanks.

Anonymous’s picture