duplicate key error while editing locale strings

related at all to http://drupal.org/node/41032
?

At least two other people are having this problem, and it seems to be PostgreSQL-specific.

See: http://drupal.org/node/47003

I can confirm this issue with Drupal 4.7 RC2 and PostGreSQL 8.1.3 on Windows and Linux.
However, the translation is stored in the database and working.

PostGreSQL automatically adds a constraint for a unique index. I don't know much about constraints yet.

The passed $form_values contains the key lid, which is processed by the statement foreach ($form_values as $key => $value), too.

So the fix for this issue is quite easy: Add a line unset( $form_values['lid'] ); to the function, after $lid.

function _locale_string_edit_submit($form_id, $form_values) {
  $lid = $form_values['lid'];
  unset( $form_values['lid'] );
  [...]

See attached patch.

New patch additionally accounting for other variable arrays 'submit' and 'form_id' in the request parameters.

It's a temporary solution. Maybe I'll find the time to fix the main cause - locale strings should be submitted in one sub-array 'strings' or the like, which is then processed.

There seems to be an spurious UNIQUE(lid) constraint in the Postgres table definition of locales_target (database.pgsql):

CREATE TABLE locales_target (
  lid int4 NOT NULL default '0',
  translation text DEFAULT '' NOT NULL,
  locale varchar(12) NOT NULL default '',
  plid int4 NOT NULL default '0',
  plural int4 NOT NULL default '0',
  UNIQUE (lid)
);
CREATE INDEX locales_target_locale_idx ON locales_target(locale);
CREATE INDEX locales_target_plid_idx ON locales_target(plid);
CREATE INDEX locales_target_plural_idx ON locales_target(plural);

while MySQL reads (database.4.1.mysql):

CREATE TABLE locales_target (
  lid int(11) NOT NULL default '0',
  translation blob NOT NULL,
  locale varchar(12) NOT NULL default '',
  plid int(11) NOT NULL default '0',
  plural int(1) NOT NULL default '0',
  KEY lid (lid),
  KEY lang (locale),
  KEY plid (plid),
  KEY plural (plural)
)

I have tried to drop the UNIQUE constraint with the Pgsql commands:

ALTER TABLE locales_target DROP CONSTRAINT locales_target_lid_key;
CREATE INDEX locales_target_lid_idx ON locales_target(lid);

Makes no difference since form data is submitted in a flat array.

array(4) {
  ["de"]=>
  string(40) "Es gibt %count Bilder in dieser Gallerie"
  ["lid"]=>
  string(4) "2410"
  ["submit"]=>
  string(17) "Save translations"
  ["form_id"]=>
  string(19) "_locale_string_edit"
}

has to be changed to

array(4) {
  ["strings"]=> array(1) {
    ["de"]=>
    string(40) "blah blah blah"
  }
  ["lid"]=>
  string(4) "2410"
  ["submit"]=>
  string(17) "Save translations"
  ["form_id"]=>
  string(19) "_locale_string_edit"
}

because a user is able to translate more than one language at one time. Whether the column lid is unique or not is unimportant. On MySQL you'll eventually find the translations "submit" => "Save translations" and "form_id" => "_locale_string_edit" in your table locales_target. I'm using PostGreSQL with the unique constraint, so they can not exist in my DB.

The nonexistent UNIQUE in MySQL I would consider as missing, thus a bug.

Patch updated based on Sun's comments. Please review.

Thanks,
-K

Smooth patch. Fixes the sub-array issue.
After some testing I found out that there's in fact a problem with the postgresql database schema. The unique constraint for locales_target is currently set up for column 'lid' but has to comprise 'locale', too. Otherwise installing/editing more than one translated language in one Drupal site is not possible, since lid is solely unique.
Both patches attached in one bundle.

Please don't ZIP your patches.

I haven't reviewed the patch, but please make sure that you submit a patch that includes database.pgsql and updates.inc.

Thanks,
-K

Hi,

Sun: Didn't read your last comment thoroughly - sorry. To submit a patch for multiple files, use the following syntax:
cvs diff -up database/database.pgsql database/updates.inc includes/locale.inc

I've rolled up a patch for updates.inc and also modified database.pgsql to mirror database.mysql.

Cvbge: Can you please check the DROP UNIQUE syntax?

Thanks
-K

Can't understand why we would like to remove the unique index. Instead I would prefer to correct the MySQL database setup to include it, too.
Why? Without unique there would be no check, if something goes wrong in locale_targets. If the unique index wouldn't be there right now, we wouldn't have seen the inadequate code and resulting wrong database entries either. Unique is there to protect the data from bad treatment. Why should we remove a protection that permits us to develop a better application?

Keep up with HEAD and changed DROP UNIQUE syntax as per Vertice's suggestion.

Sun: Please open a separate issue for that. And I don't think that it should be UNIQUE - it should probably be a composite primary.. and these things bear investigation. I also think that the other indices (in all of Drupal's tables leave alone the locale tables) could also do with some attention.

Thanks,
-K

There was some talk about the table to be slow, and the index needing to be adjusted (so we don't index the entire string). I can't find the issue right now, but it might be relevant. I'd try to find it before we drop that unique.

Opened a new issue for the key changes: http://drupal.org/node/63561

Dries: The issue you are thinking of relates to locales_source where the source BLOB is indexed. Unrelated to this table. The DB changes in this issue just make the pgsql table congruent to the equivalent mysql table.

Thanks,
-K

Dries: this one? http://drupal.org/node/42463

Zen: yep, #42463 was the one. I'll commit this patch then. Thanks for looking this up! :)

Committed this patch to CVS HEAD.

I have committed the changes to locale.inc to 4.7. I need to figure out how to handle database updates (the numbers of the updates) before committing the pgsql fix.

Drumm, please elucidate.

I hadn't thought of a 100% solution for branched updates yet. We can safely skip update version numbers, so the simplest way that theoretically works is to make a set of reserved update numbers which HEAD doesn't use so that they may be filled in by 4.7 as needed. Of course, HEAD already used some. Creating any more complex systems is up for debate. Tip: use variables to track inconsistencies if needed.

I don't see an active patch, and seems like this might be critical for 4.7.

So, essentially, I need to apply the last three database updates that went into HEAD to 4.7 too?

4.7 is now at 179 while HEAD is at 182.

I don't have a problem with 180 and 182 is the one I need to have anyway, but 181 is a new feature and 180 also lacks the pgsql part.

I guess I could live with the database change of 181 and simply don't change the profile module's code.

Can we maybe agree that the next database update on HEAD should be 200? I think we need a better versioning scheme here in the medium run. Can we change schema_version to something else but smallint(3)? Maybe decimal(3,2) so we can use the .xy for versioning?

alter table system change schema_version schema_version decimal(3,2) NOT NULL DEFAULT -1;

seems to work.

complete database update was committed to 4.7.