Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#1584 closed defect (fixed)

[patch] auto_now_add fields set to NULL on update

Reported by: Andy Dustman <farcepest@…> Owned by: Adrian Holovaty
Component: Database layer (models, ORM) Version: master
Severity: normal Keywords:
Cc: farcepest@…, fawad@…, upadhyay@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When adding an object with a models.DateTimeField(auto_now_add=True), the column is set to the current time as it should be. However, when doing an update, Django tries to set the column to NULL.

Example: These are SQL statements (and parameters) as they are executed. created is auto_now_add and updated is auto_now. First, adding the new objects in the admin interface:

INSERT INTO `dns_domain` (`name`) VALUES (%s) ['foo.com']
INSERT INTO `dns_nameserver` (`domain_id`,`name`,`ip`,`starts`,`expires`,`ttl`,`location_id`,`created`,`updated`) VALUES (%s,%s,%s,%s,%s,%s,%s,%s,%s) [2L, 'a', '1.2.3.4', None, None, None, None, '2006-04-04 17:50:40', '2006-04-04 17:50:40']
INSERT INTO `dns_nameserver` (`domain_id`,`name`,`ip`,`starts`,`expires`,`ttl`,`location_id`,`created`,`updated`) VALUES (%s,%s,%s,%s,%s,%s,%s,%s,%s) [2L, 'b', '1.2.3.5', None, None, None, None, '2006-04-04 17:50:40', '2006-04-04 17:50:40']

Then, updating the objects in the admin interface:

SELECT `dns_domain`.`id`,`dns_domain`.`name` FROM `dns_domain` WHERE (`dns_domain`.`name` = %s) ['foo.com']
SELECT 1 FROM `dns_domain` WHERE `id`=%s LIMIT 1 ['2']
UPDATE `dns_domain` SET `name`=%s WHERE `id`=%s ['foo.com', '2']
SELECT `dns_nameserver`.`id`,`dns_nameserver`.`domain_id`,`dns_nameserver`.`name`,`dns_nameserver`.`ip`,`dns_nameserver`.`starts`,`dns_nameserver`.`expires`,`dns_nameserver`.`ttl`,`dns_nameserver`.`location_id`,`dns_nameserver`.`created`,`dns_nameserver`.`updated` FROM `dns_nameserver` WHERE (`dns_nameserver`.`domain_id` = %s AND `dns_nameserver`.`id` = %s) [2L, '4']
SELECT 1 FROM `dns_nameserver` WHERE `id`=%s LIMIT 1 ['4']
UPDATE `dns_nameserver` SET `domain_id`=%s,`name`=%s,`ip`=%s,`starts`=%s,`expires`=%s,`ttl`=%s,`location_id`=%s,`created`=%s,`updated`=%s WHERE `id`=%s ['2', 'a', '1.2.3.4', None, None, None, None, None, '2006-04-04 17:55:02', '4']
SELECT `dns_domain`.`id`,`dns_domain`.`name` FROM `dns_domain` WHERE (`dns_domain`.`id` = %s) ['2']
SELECT `dns_nameserver`.`id`,`dns_nameserver`.`domain_id`,`dns_nameserver`.`name`,`dns_nameserver`.`ip`,`dns_nameserver`.`starts`,`dns_nameserver`.`expires`,`dns_nameserver`.`ttl`,`dns_nameserver`.`location_id`,`dns_nameserver`.`created`,`dns_nameserver`.`updated` FROM `dns_nameserver` WHERE (`dns_nameserver`.`domain_id` = %s AND `dns_nameserver`.`id` = %s) [2L, '5']
SELECT 1 FROM `dns_nameserver` WHERE `id`=%s LIMIT 1 ['5']
UPDATE `dns_nameserver` SET `domain_id`=%s,`name`=%s,`ip`=%s,`starts`=%s,`expires`=%s,`ttl`=%s,`location_id`=%s,`created`=%s,`updated`=%s WHERE `id`=%s ['2', 'b', '1.2.3.5', None, None, None, None, None, '2006-04-04 17:55:02', '5']
SELECT `dns_domain`.`id`,`dns_domain`.`name` FROM `dns_domain` WHERE (`dns_domain`.`id` = %s) ['2']

Note that created is being set to None/NULL.

Example done with mysql backend. The problem seems to lie in models itself and is not specific to the admin interface: I could update an object using the shell and it would do the same thing. I'm assuming "Database wrapper" encompasses django.db.models and not just the backends.

Attachments (3)

auto_now_add_fix.diff (546 bytes) - added by Andy Dustman <farcepest@…> 10 years ago.
Bug fix (against magic-removal)
auto_now_add_fix.2.diff (546 bytes) - added by Andy Dustman <farcepest@…> 10 years ago.
Better bug fix (evaluation order matters)
auto_now_add_fix.patch (3.8 KB) - added by Luke Plant 10 years ago.
Patch involving reworked Field.pre_save()

Download all attachments as: .zip

Change History (19)

comment:1 Changed 10 years ago by anonymous

Cc: mir@… added

Changed 10 years ago by Andy Dustman <farcepest@…>

Attachment: auto_now_add_fix.diff added

Bug fix (against magic-removal)

Changed 10 years ago by Andy Dustman <farcepest@…>

Attachment: auto_now_add_fix.2.diff added

Better bug fix (evaluation order matters)

comment:2 Changed 10 years ago by Andy Dustman <farcepest@…>

Summary: auto_now_add fields set to NULL on update[patch] auto_now_add fields set to NULL on update

Patch available

comment:3 Changed 10 years ago by anonymous

Cc: fawad@… added

comment:4 Changed 10 years ago by anonymous

Cc: upadhyay@… added

comment:5 Changed 10 years ago by fawad@…

I'm seeing that this problem happens on the current Django trunk as well.

class NotificationSchedule(models.Model):
    class Admin:
        pass
    def __str__(self):
        return "%s (%s hours before the event)" % (self.name, self.offset)
    name = models.CharField("Description of the schedule", maxlength = 72, core = True)
    offset = models.FloatField("Number of hours before event the notifications should be sent", core = True,
        decimal_places = 2, max_digits = 10)
    created = models.DateTimeField(auto_now_add = True)
    modified = models.DateTimeField(auto_now = True)
[fawad@fawad5 meetup]$ python manage.py shell --plain
Python 2.5a1 (trunk:45386, Apr 14 2006, 09:22:06)
[GCC 3.4.4 20050721 (Red Hat 3.4.4-2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from meetup.models import NotificationSchedule
>>> x=NotificationSchedule(name="foo", offset=3)
>>> x.save()
>>> x.offset=4
>>> x.save()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/stow/python-cvs/lib/python2.5/site-packages/django/db/models/base.py", line 169, in save
    db_values + [pk_val])
  File "/usr/local/stow/python-cvs/lib/python2.5/site-packages/django/db/backends/util.py", line 12, in execute
    return self.cursor.execute(sql, params)
  File "/usr/local/stow/python-cvs/lib/python2.5/site-packages/django/db/backends/sqlite3/base.py", line 73, in execute
    return Database.Cursor.execute(self, query, params)
IntegrityError: meetup_notificationschedule.created may not be NULL

comment:6 Changed 10 years ago by Andy Dustman <farcepest@…>

Cc: farcepest@… added
milestone: Version 0.92
Version: magic-removalSVN

With the demise/merging of magic-removal, this is still an issue in the trunk. Presumably other people who are watching this bug would have spoken up if the patch didn't work, and it's simple enough that it's "obviously" correct (for sufficiently large values of obvious).

comment:7 Changed 10 years ago by asmodai@…

This is biting me as well on trunk. Using a a FileField in conjunction.

The patch does not seem to work for me though. Although someone on IRC did solve his problem with it. Leaves me to assume the issue runs a bit deeper.

I see that after a save in the Admin gui the failing SQL is an UPDATE with the date field set to None on the update, which of course fails the constraint and gives me an IntegrityError submit_date may not be NULL.

submit_date is: models.DateField(auto_now_add = True)

comment:8 Changed 10 years ago by asmodai@…

The person on IRC noted that his constraint was removed. It also fails for him.

comment:9 Changed 10 years ago by mdt@…

i had to change the line in question (line 421 in 3001) to

if self.auto_now or (self.auto_now_add and (add or value is None)):

which works for me. i dont understand why pre_save() just returns the value without setting it at the instance...

comment:10 Changed 10 years ago by Luke Plant

The patch doesn't exactly fix it: if you save an object n times, the datetime value will be set to the datetime at the last time you saved, not the first. The problem is that the value is not being saved to the object. The pre_save method cannot currently do this, since it doesn't have access to the object (pre_save is a method on a Field object and Field objects are essentially definitions that don't contain data -- the data itself is a member of the model class instance).

The reason this is not normally a problem is that you normally discard' an object after saving it -- the next time it is used will be by another request, which will load it fresh from the database, and the datetime value will be set.

The easiest way to fix it, as I see it, is to change the arguments of Field.pre_save() so that it takes a model instance instead of a value. I'll come up with a patch and check it with Adrian.

Changed 10 years ago by Luke Plant

Attachment: auto_now_add_fix.patch added

Patch involving reworked Field.pre_save()

comment:11 Changed 10 years ago by Luke Plant

Resolution: fixed
Status: newclosed

(In [3002]) Fixed #1584 - auto_now_add fields now work when saving multiple times.

comment:12 Changed 10 years ago by Luke Plant

As you can tell, I went ahead and committed the patch -- I was pretty sure about it. The only issue might be compatibility with people who have subclassed Field, but as I understand it that's an internal class -- subclassing Field isn't documented anywhere.

comment:13 Changed 10 years ago by dave@…

Resolution: fixed
Status: closedreopened

Even with the patch, when updating edit_inline fields, null is set (individual editing of the same model has been fixed.) Had to override save() for inline editing of models with auto_now_add.

comment:14 Changed 10 years ago by Jacob

Resolution: fixed
Status: reopenedclosed

Reclosing -- edit_inline is broken in more ways than this, and is being rewritten.

comment:15 Changed 10 years ago by mir@…

Cc: mir@… removed

Removing myself from Cc.

comment:16 Changed 10 years ago by Adrian Holovaty

milestone: Version 0.92

Milestone Version 0.92 deleted

Note: See TracTickets for help on using tickets.
Back to Top