" Duplicate entry 'F j, Y - H:i' " warning for INSERT into date_format table

The Date module is trying to insert a default date format that is the same as an existing value. (Likely a custom format.)

If you update the database manually so the indicated format matches the values in the insert query, the warning should disappear.
(If it's a custom format, replace custom with medium and 0 with 1.)

It should be possible to modify the module to check for custom formats that match a format the Date module is trying to insert into the database and update the entry instead of adding a new one. I'm just not sure where to put it. I managed to track the problem to the date_format_save() function in Date API, but I couldn't get the table to update when the new format matched an existing custom format. There's something going on that I'm not seeing.

updating the database manually did fix the problem. Thanks.

I found out what I wasn't seeing before. The module was updating the custom formats last. As a result, the format that I updated from a custom type to one of the module defined types would be overwritten with the original values.

I have created a patch that allows the module to update custom formats that are now defined by a module. During this initial update, additional attempts to update the format are suppressed, keeping it from reverting back to a custom format right away.

I can't reproduce this issue. The patch attached should work, however I feel that it doesn't fix the underlying problem and may be adding unnecessary code to date_format_save().

@Cybergarou does it only happen for you when update.php is run as ben_a suggests in the original post? When upgrading from last RC release to latest dev, you need to run update.php twice - on which instance does it occur on? Can you provide any steps to reproduce this?

Cheers,
Stella

I saw the warning during both runs of update.php and I also saw it anytime cron was run or the cache was cleared.

With a clean installation of the date module, all you need to do to reproduce this is change the value of the type field in the date_format table to something else. You will get the warning anytime the table is rebuilt.

I'm still trying to figure out what's happening in the relevant functions, but at this point I have to agree that my earlier patch was the wrong approach. It appears to me that the problem is related to how the list of date formats is built. An array is built using both the format and the type as keys. This means that whenever the format type is changed it shows up as a new format and not as an update.

I made another patch that strips out the type key. A few lines had to be removed from _date_format_build() to get the table updating properly. I also modified date_get_formats() to maintain the type filtering lost by removing the type key. Someone familiar with how these functions work will have to take a look to make sure these changes don't break anything, but as far as I could tell they still produce the expected results.

If you post your new patch I'll take a look.

Steps to reproduce - is there any way to reproduce other than modifying the new 'date_format' table values directly? How can this issue be reproduced in the normal course of operations of the module?

The issue occurs during normal operations if you are unlucky enough to have a custom format that matches a format being introduced by the system. If you add a custom format that matches one of the new system formats (like the one in the issue title) before upgrading you should see it.

The new patch needs work. I found it breaks the use and display of formats. Changing the structure of the array had more impact than I expected. I'm working on cleaning up the patch so that less of the module is affected.

I haven't actually tried this out, either trying to reproduce the problem or your patch to fix it, but if the only way the problem will occur if you try to create a new format that matches an existing format, can't we just catch and disallow that in the form validation when you try to add a new format? Then we can leave the rest of the code alone. It feels like this patch makes the fix harder than it needs to be.

If you manually alter the table and leave duplicates in there, I'm not sure I think it's the Date module's job to clean up your mess, but if you try to create a duplicate in the UI (which would be an easy thing to do) we should take care to keep that from happening.

We would also need to check for that situation in the update function, so that needs to be in the patch.

The problem is that it is the module trying to add duplicate formats, not the user. When the module builds the list of formats it can't see that format X with type Y is the same as Format X with type Z.

Here is the "cleaned up" patch. It avoids breaking the rest of the module by adding the type key back into the array.

I don't know if it would be best to remove the need for that key or not, but I want to keep my changes localized to a small part of the module. I'll leave that decision to KarenS.

Ok here's a patch which doesn't fix the issue :) It fixes a similar issue which happens on the 'add format' page when a duplicate format is attempted to be added. It also contains a fix for errors about the 'date_format_types' table not being available during update.php because date_theme() is being called during update.php, after the files have been upgraded, but before the database has. Might be only an issue with newer versions of Drupal 6, because this wasn't happening before.

@Cybergarou again I'm not totally happy with the direction of this patch, sorry. Unfortunately I still can't reproduce it. Obviously if I modify the values in the db directly I can, but not through the normal operation of the module. I'd like to understand why / how custom formats are being inserted into the database before the module defined ones. Can you provide more detailed instructions on how to reproduce it? What version of 'date' are you upgrading from? When you say 'custom formats', where exactly are you setting these - on admin/settings/date-time or on a CCK date field?

Ooops here's the patch. Should be merged into the final version of the other one.

@Cybergarou just to clarify, there are good points in your last patch, just not sure about all of it, need to consider it further.

Ok, even though I couldn't reproduce the issue described above, I came up with a couple of other scenarios where it would be possible to get the same error. Having talked to Nedjo on IRC, I realised we're actually going about fixing this totally the wrong way. We've been trying to fix the code, whereas really we need to fix the database to match the desired functionality. So rather than the date format string being unique over all format types, it is now only unique per format type. I've added these changes to my last patch above and have attached the final version. This patch has the following changes:

• Database structure had to change so the unique key is actually a combination of 'format' and 'type', and not just 'format'. I also had to reduce the column sizes (which are still more than large enough) as MySQL has some restriction that the key size should be no more than 1000 bytes key size = (3x (sizeof 'type' + sizeof 'format')). I didn't provide a hook_update_N() for this as it's not released yet, but can easily do this if you want Karen.
• Updated date/date.install file so the hook_update only checks that the format is unique per type, rather than unique overall when inserting custom formats.
• Updated date_api.admin.inc file so the uniqueness check on the format string is based on the format type too.
• Included fix for date_theme() trying to use the date_format_types table during update.php before the table had been created.
• Updated date_format_save() so it updates correctly based on new table keys.

I've done some testing of this (but considering it's gone 2am here, probably not enough), including the new date_locale module too, and didn't encounter any problems. However it'd be great if others could test this too.

Cheers,
Stella

@stella, thanks! I know that there are people who have already updated to this code so we'll need the hook_update, too. I'll do some testing later today.

Just to be clear here, I've only directly modified the datebase trying figure out what's going on here. I never touched the tables before I got this warning. Any modifications I made since were to return the tables to the state they were in when I noticed the issue.

As for where the custom formats came from, I wish I could tell you. I never knowingly added any custom formats. I went through my database backups and determined they were populated in the table when it was created on Feb. 7. This was the last time I updated to the latest dev version. The previous dev version I was using was obtained on Jan. 23. At that point, the extent I used the module for was to provide date fields in CCK.

There are things about my patch I also don't like. My changes to date_get_formats() can only be described as a hack. Its purpose is to keep the modified function working with the rest of the module. A proper integration of my changes to _date_format_build() would require widespread changes of functions I don't understand.

I'm going to have to disagree with the database change though. Making the formats unique only to type means multiple copies of formats will be possible. There are two problems with that.

• Any select list that doesn't filter by type is going to have multiple copies of some formats listed.
• It kind of defeats the purpose of having types in the first place. If a format gets listed under almost every type, why bother?

Instead of treating format as a property of type, type should be a property of format.

Date format strings added by modules are just suggestions for that date format type, and as such we really should allow modules to suggest format strings that my already be defined by a different module for a different type.

With what you're suggesting we could actually end up with no format string suggestions making it into the database for a particular module defined format, if it just so happens that another module had already suggested those strings for a different format type. This means none of those valid suggested formats would be available to that module, which could result in potentially serious errors in that module's operations.

Any correctly implemented select list is going to be of the style $options[$format] = date_format_date(date_now(), 'custom', $format);, so the key is the format string and human readable option is the value. Since the key is the format string, duplicate copies in selects is a non-issue.

I wasn't aware that getting the values for the select lists worked that way. My second point still stands though.

Modules shouldn't be looking for the format type, the only thing that should matter to them is that the desired format is present. The only reason I can think of for allowing modules to define formats for different types is to give the module ownership of the format. Only Date API should have ownership of formats.

@Cybergarou the main objectives of the date formats section of date_api module are:

• to allow users to define custom format types, example use case is with cck date fields.
• to allow users to define custom format date strings
• to allow other contrib modules to define their own date format types, for example an 'event' type module may not want to use 'short', 'medium' or 'long' formats and may need their own 'event' date format type.
• to allow other contrib modules to define date format strings for their defined format types, and for date_api provided types.
• to have the module be extensible as possible
• to allow the formats be localizable if date_locale module is enabled

These features, along with date_locale module, were added to the module in #318008: locale based handling and rendering

I didn't mean to make it sound like what you are doing is wrong, I'm sorry about that. I'm just worried about some problems that I'm seeing. Some of the things you are adding to the module are features from your viewpoint, but from my viewpoint, somebody who just needs basic date fields, those features are bugs. The only reason I'm taking this much interest in the matter is that I have found myself considering writing a module to REMOVE features from a core date module. I feel that when that happens something is off.

I guess the root of my problems with what you are doing involve how the type field is being used. In one place it is a way of categorizing the formats. Elsewhere it is a link between formats and modules. From my viewpoint a format shouldn't show up in multiple places. If a user adds a format it should be in the list of custom formats where it can be deleted. If a module defines a format it should be moved or added to one of the other lists where it can't be deleted. When a module stops defining a format it should be moved to the list of custom formats so the user can choose to delete it. Using type as a linking method breaks this. (Though truthfully it doesn't happen anyway.)

I'm going to make a suggestion that I hope will be able to make us both happy.

• Use the type field strictly to define what type a format is, 'custom', 'short', 'medium', or 'long'. (I can't understand how a format can't be one of these, but you could add 'other' as well.)
• Then make a new field 'modules' to store what modules use the format.
• Treat the modules entries like tags so that multiple modules can link to a format, but we don't need to store multiple versions of the same format.
• When a format no longer has any modules 'tagged' to it, the format can be moved to the custom list.

You may have noticed an emphasis on users deleting formats and not modules, let me explain with an example. The way things are I believe the following problem is possible. (And forgive me if you have already accounted for this.)

1. Say module A defines format 'Y' with type 'special'.
2. The user makes some posts that use this format.
3. Now module B is added which defines a bunch of formats, including format 'Y' with type 'special'.
4. After a while the user feels module B isn't needed and uninstalls it.
5. Module B deletes all of it's formats including format 'Y' with type 'special'.
6. Module A recreates format 'Y' with type 'special', but now the dfid is different.

All previous date displays using the old dfid are broken now and the user doesn't understand why because the format is still listed. If the user has to delete the format they understand what happens.

@Cybergarou sorry, didn't mean to sound defensive. Just wanted to make it clear that having extensible date format types / strings is something that is we intended and is by design.

Just to clarify, any format strings and format types defined by a module are flagged as 'locked' so they can't be edited or removed by the user via the UI.

The issue you describe shouldn't be possible because when a module is disabled, none of their format types / strings are removed from the database. However they will no longer be flagged as "locked", essentially becoming user defined formats. By the way, if the module is re-enabled it won't result in duplicates, as the code will recognise we already have that information, and will only update it (if necessary) and will mark it as a "locked" type again.

In addition, the date format string configured for a particular date format type is stored as a variable in the variables table and is accessed via variable_get(). For example, if you have a date format string Y-m-d and the type special, the 'Y-m-d' is the value of the variable date_format_special. At no point is the dfid used when accessing the configured format string for a particular type. So the date will continue to display correctly until such time as you edit your post and save it with an available date format type/string.

The dfid is only used for things like the 'remove' link, e.g. 'admin/settings/date-time/formats/delete/40' It's used here to avoid the issue you describe - so you only delete the specific instance of the format string and not all instances.

Cheers,
Stella

That explains why many of the functions don't work the way I would expect them to. If it wasn't obvious, we have very different design paradigms.

Normally I wouldn't use the variable table in this way, but seeing as the number of variables that would end up in it by adding format types is completely reasonable I'm not going to make a fit. Just be sure to add something to date_api.install to remove those variables in the unlikely event it's uninstalled. (That's another issue, I'll leave it to you.)

As for the main issue, since you have assured me that I'm seeing problems that aren't really problems, I've tested the patch in #12 and it solves the table entry issue. I didn't encounter any errors caused by the patch, keeping in mind that my use of this module is limited to the basics.

We have to use the variable table, at least for 'long', 'medium' and 'short' types because that's where core saves that information.

Patches and things aside, it is possible to remove the warning by following the instructions here: http://drupal.org/node/386806#comment-1314694

Hope that helps some out there!

Subscribing....

Hi
Also got this. My db have been running on D4.7.x D5 and now D6.9. I have not manually changed/added any special formats.

* user warning: Duplicate entry 'D, m/d/Y - H:i' for key 2 query: INSERT INTO date_format (format, type, locked) VALUES ('D, m/d/Y - H:i', 'medium', 1) in drupal/includes/common.inc on line 3425.

* user warning: Duplicate entry 'l, F j, Y - H:i' for key 2 query: INSERT INTO date_format (format, type, locked) VALUES ('l, F j, Y - H:i', 'long', 1) in drupal/includes/common.inc on line 3425.

@steinmb: have you tried the attached patch? Though the database structure has changed, and the patch doesn't yet include a database update bit.

Stella

Hi all -

I implemented the patch in #12 but am still getting the same error. I disabled all of the date-related modules, flushed cache, and then re-enabled all of the date modules. The enabling was errorless, but when I ran update.php I got the error again. (I upgraded from the old .dev version to the latest .dev today).

My error is as follows: user warning: Duplicate entry 'm/d/Y - g:ia' for key 2 query: INSERT INTO date_format (format, type, locked) VALUES ('m/d/Y - g:ia', 'short', 1) in /home/public_html/includes/common.inc on line 3433.

Any ideas on how to resolve this?

Yeah the patch itself doesn't fix the issue if you've already upgraded the database. A database upgrade path from the dev version to the patched version still needs to be implemented.

Doh, thanks stella, that's what I feared... any idea when a path will be implemented?

There's no warning of the potential conflict... if it's been a known issue for a few weeks now shouldn't it be rolled or at least have a warning put on the project page to implement the patch first - or is it just not worth it?

Most people will find that their site operates just fine without this patch. They will just see warnings. Thus they don't have to worry about applying this patch. The only people who might have to worry are the ones that depend on having date formats based on locale.

The only reason the issue hasn't been resolved yet is the patch hasn't received the testing it deserves. I only make light use of the module so I'm unlikely to encounter any errors. To really say the patch is tested, somebody who uses locales should take a look at it. The database change is really simple so don't let the fact that there's no update path yet scare you off.

Manually updating date_format worked for me.

@stella: Sorry I have not yet had time to properly test your patch. I hope I'll find time before I leave Sunday morning, otherwise it have to wait until I'm back by the end of next week.

I can confirm this error as well. It happens to me every time I clear the cache, but only on one of the two sites I've updated to date 2.x-dev from 8 March. The other one is fine. For now I'm fixing it by deleting the old input format, which seems to work, and hoping it doesn't break anything else. The sites I have are too close to launch to be playing around with experimental database upgrades.

I've committed the fixes and an update to update the table changes. I didn't try to write an update path to find duplicate formats. I'm pretty sure that manually deleting any you find will work sufficiently and I don't want to be more aggressive than necessary on the update.

I went back and did a fresh install of a pre-format-change site, updated it to the problem status and then did another update with the latest patch and everything updated without errors. I was never able to see all the errors everyone else reported. The only place I saw an error was when I manually tried to enter a duplicate format, and that error was fixed by the patch.

So I'm going to mark this fixed.

Thanks Karen, I manually deleted the custom entry duplicate and reran update.php, fixed my problem w/o issue.

Thanks!

I can confirm that upgrading from 2.0 to 2.1 (well, 2.x-dev of 13 April actually, but that should not be material) fixed this error in my case. Many thanks Karen!

subscribing