locales_source.location is just varchar(255) and it is not truncated

Well, we could strip the string in Drupal and only save a 255 char long substring. The location field is not designed to provide you with accurate information. It only collects the first place the string occurred, which could be quite random. Also, if you import a .po file, it could have several lines of occurrence information, which is also truncated. I understand this causes an error in postgresql, so I'd say we should trim the value. (Drupal 5 and 6 would not receive database changes anyway).

We do not avoid database changes in Drupal 7, we do avoid database changes if at all possible with stable versions (Drupal 5 and 6) to keep compatibility with existing modules.

This bug targeted both d6 and d7 same as #107824: Convert {watchdog}.referer and {accesslog}.url from VARCHAR to TEXT

Quick patch for both branches

While the location field is fairly arbitrary for the 'default' textgroup (managed by core's t() function), and so could be safely truncated, it's used for other purposes in contrib for other textgroups. The i18nstrings module uses it to concatenate an array of keys identifying a string. I don't know of any cases where the resulting string is longer than 255 characters, but if it was we couldn't truncate without losing data.

@nedjo a lot of short strings already longer, for ex

select * from locales_source where source = 'delete';

returns:

includes/locale.inc:86,2006; modules/block/block.admin.inc:79; modules/book/book.admin.inc:238; modules/comment/comment.module:826; modules/contact/contact.admin.inc:16; modules/filter/filter.admin.inc:35; modules/menu/menu.admin.inc:100; modules/node/co

which as you can see truncated by mysql

so there are only 2 solutions - truncate (now) or make this field text-type as #107824: Convert {watchdog}.referer and {accesslog}.url from VARCHAR to TEXT

Let's go with a TEXT field.

Suppose first should be patched 6-dev version then follow-up for D7

tracking cvs HEAD

Strange syntax error - patch is trivial so next review

ps last comment lost body and status change

It seems that the testbot missed this issue. I've submitted a retest of the last failed patch as I can't see it causing a syntax error.

So here is reroll

Another reroll after #473366: DBTNG locale.module

Committed to CVS HEAD. Thanks. Lowering version.

from IRC 22:45 Dries__: andypost: in d6, we just want to truncate the url, imo, so we don't have to change the schema

So here the old patch

I have to agree with Damian on this one. We should go with a TEXT field here rather than cutting the location short, there is no point in having a location if you can't use it to locate anything. Once this is done we can do what we did over at #107824: Convert {watchdog}.referer and {accesslog}.url from VARCHAR to TEXT and write a patch for 7 to support the upgrade.

So waiting Gábor Hojtsy's review!

There is a same problem on D6's locale.module:378;
When using OpenID module with yahoo.com which returns over 255 byte uri, this part of code occurs error.

--- locale.module.orig	2009-06-19 00:39:42.000000000 +0900
+++ locale.module	2009-06-19 00:40:27.000000000 +0900
@@ -378,7 +378,8 @@
     }
     else {
       // We don't have the source string, cache this as untranslated.
-      db_query("INSERT INTO {locales_source} (location, source, textgroup, version) VALUES ('%s', '%s', 'default', '%s')", request_uri(), $string, VERSION);
+      $location = drupal_truncate_bytes(request_uri(), 255);
+      db_query("INSERT INTO {locales_source} (location, source, textgroup, version) VALUES ('%s', '%s', 'default', '%s')", $location, $string, VERSION);
       $locale_t[$langcode][$string] = TRUE;
       // Clear locale cache so this string can be added in a later request.
       cache_clear_all('locale:', 'cache', TRUE);

So here fix with changing table locales_source.location to TEXT

location size could be very long see http://drupal.org/node/107824#comment-1693226

Let's make a Db change in d6

Same path but for D6.13

@sage don't change version

there are 2 ways - truncate string and change database field to TEXT type

I'm going with andypost's patch here.

This is not in fact in Drupal 7 anymore due to #278592: Sync 6.x extra updates with HEAD, which removed D5 -> D6 update functions. There were two issues with andypost's patch which resulted it being removed:

1. It was a new 6xxx update function committed to Drupal 7. A big no-no. If you aim for Drupal 7, add 7xxx update functions.
2. The 6xxx update function was in the 5.x to 6.x update group, which is inevitably removed in D7. It should be in a 6-extra group in D6 even (and a 7.x group in 7.x, see (1)).

Similar reasons necessitated a follow up patch in http://drupal.org/node/107824#comment-2038438 for example.

So let's get this back again in Drupal 7 and then 6 (and then update 7 again).

I've got this issue on Drupal 6.14 with MySQL 5.x after enabling a theme, too. This is not limited to PostgreSQL. Removing wrong tagging.

user warning: Data too long for column 'location' at row 1 query: UPDATE locales_source SET location = ' theme-settings.inc:128,158,180; yaml.info:0; layouts/yaml_1col/yaml_1col.info:0; layouts/yaml_2col_13/yaml_2col_13.info:0; layouts/yaml_2col_31/yaml_2col_31.info:0; layouts/yaml_3col_standard/yaml_3col_standard.info:0; layouts/yaml_3col_subcol/yaml_3col_subcol.info:0' WHERE lid = 1011 in D:\InetPub\mysite\includes\locale.inc on line 1353.

D7 branch have comment but actually no update happen!

@Gábor Hojtsy #334283: Add msgctxt-type context to t() is raised for upgrade path, so let's make a change in D6 and then change D7 upgrade path

So I leave this postponed till is not fixed #334283: Add msgctxt-type context to t()

Now patch for D6 with changed schema (forget in previous)

So this could be commited to 6

Location was lost again

taggin

#33 should wait til #278592: Sync 6.x extra updates with HEAD

Upgrade path for D7 locale_update_7000() fixed in #278592: Sync 6.x extra updates with HEAD

So lets solve this for D6!

This error caused by mysql settings, some systems have this by-default

http://dev.mysql.com/doc/refman/5.0/en/server-sql-mode.html#sqlmode_stri...
http://dev.mysql.com/doc/refman/5.0/en/server-sql-mode.html#sqlmode_stri...

Let's backport this change because D6 supports mysql 5.0

patch from #38 still valid

+++ modules/locale/locale.install	17 Oct 2009 18:07:43 -0000
@@ -396,3 +395,22 @@ function locale_schema() {
+/**
+ * Allow longer location.
+ */
+
+function locale_update_6006() {

Pretty sure that you shouldn't leave a blank line between PHPdoc and function. Other than that, looks good to go.

I'm on crack. Are you, too?

@Arancaytar thanx a lot for review!

Fixed patch.

Patch Failed for me.
I'm using Drupal 6.13.

Here's the error:

user warning: BLOB/TEXT column 'location' used in key specification without a key length query: ALTER TABLE locales_source CHANGE location `location` LONGTEXT DEFAULT NULL in C:\Inetpub\wwwroot\Politics_dep\includes\database.mysql-common.inc on line 520.

...
The following queries were executed
locale module
Update #6006

    * Failed: ALTER TABLE {locales_source} CHANGE location `location` LONGTEXT DEFAULT NULL

This is a Mysql related error. Here's the solution I found (from here):

The error happens because MySQL can index only the first N chars of a BLOB or TEXT column. So The error mainly happen when there is a field/column type of TEXT or BLOB or those belongs to TEXT or BLOB types such as TINYBLOB, MEDIUMBLOB, LONGBLOB, TINYTEXT, MEDIUMTEXT, and LONGTEXT that you try to make as primary key or index. With full BLOB or TEXT without the length value, MySQL is unable to guarantee the uniqueness of the column as it’s of variable and dynamic size. So, when using BLOB or TEXT types as index, the value of N must be supplied so that MySQL can determine the key length. However, MySQL doesn’t support limit on TEXT or BLOB. TEXT(88) simply won’t work.

The solution to the problem is to remove the TEXT or BLOB column from the index or unique constraint, or set another field as primary key. If you can’t do that, and wanting to place a limit on the TEXT or BLOB column, try to use VARCHAR type and place a limit of length on it. By default, VARCHAR is limited to a maximum of 255 characters and its limit must be specified implicitly within a bracket right after its declaration, i.e VARCHAR(200) will limit it to 200 characters long only.

What would you suggest I do?

Thanks, Nimi.

@nimi Please check keys for your {locales_source} table first. As you can see http://api.drupal.org/api/function/locale_schema/6 there's only 2 keys (lid and source).

Suppose your installation have modified DB

Marked as duplicate #279219: Grave bug: MySQL trims and destroys data

I solve the problem for the 221020_locale_location_D6_2.patch with this.

<?php
/**
 * Allow longer location.
 */
function locale_update_6006() {

  $ret = array();
  $i18n = db_drop_index($ret, 'locales_source', 'textgroup_location');

  if ( $i18n !== FALSE ) {

    db_change_field($ret, 'locales_source', 'location', 'location', array('type' => 'text', 'default' => NULL));
    db_add_index($ret, 'locales_source', 'textgroup_location', array(array('textgroup', 30)));
  } else db_change_field($ret, 'locales_source', 'location', 'location', array('type' => 'text', 'default' => NULL));
  
  return $ret;
}

/**
 * @} End of "defgroup updates-6.x-extra"
 * The next series of updates should start at 7000.
 */

?>

I'm intrigued at how @killua99 and @nimi had the same issue with the DB independently? What would add the textgroup_location index? i18nstrings.module? (Which actually uses the location and textgroup fields to look up stuff, so would be logically indexing them). I'm reluctant to commit this change due to the potential to break stuff as it showed by two reviewers already. Can we figure out why this breaks for some people even though the core schema does not have an affected index?

I file an issue #803380: locales_source.location index

Another reason to commit this a _locale_rebuild_js() which query uses WHERE s.location LIKE '%%.js%%' so location could be truncated and we never get JS-strings translated

Also there's amazing related issue #504506: Drupal.formatPlural incorrectly handle complex plural rules

There's a patch waiting for review in i18n queue #803380-1: locales_source.location index
If this fixed we could proceed with D6 fix

Also there's a discussion about plurals which could be fixed #532512: Plural string storage is broken, editing UI is missing

Removing D7 upgrade path tag

Subscribe.

Ran today in this issue too with Drupal 6.22 when trying how Drupal handles OpenID failed login - similar to @miurahr in #24

Just wondering why this isn't fixed in D6 for more than two years being classified as critical... See in attchment how nice is the localized D6 user register page after failed Openid Login...

Marked #232589: "Data too long for column" and "Duplicate entry" a duplicate.

Subscribing

Subscribing to this as well. Any progress?

rerolled patch against head
also I've added instruction to drop i18nstring's indexes on location_source table as of it causes update fail

what happens if the module doesn't exists?

+++ b/modules/locale/locale.install
@@ -239,6 +239,21 @@ function locale_update_6007() {
+  if (module_exists('i18nstrings')) {
+    // i18nstrings should manually care about its indexes
+    db_drop_index($ret, 'locales_source', 'textgroup_location');
+  }

what happens if the module doesn't exists?

then all will be ok

I don't know how to solve right problem with i18nstrings
May be someone can suggest something ?

Hi,

Will it be merged into official repo? I'm facing this issue in my D6 installation.

#57: 221020_locale_location_D6.patch queued for re-testing.

#35: 221020_locale_location.patch queued for re-testing.