Opened 16 years ago

Closed 15 years ago

#5903 closed (fixed)

DecimalField returns default value as unicode string.

Reported by: justin.driscoll@… Owned by: Brian Rosner
Component: Database layer (models, ORM) Version: dev
Severity: Keywords: DecimalField
Cc: pigletto@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Until a model instance is saved Django returns a default DecimalField value as a unicode string. This creates an ambiguity as to the return value when it is unknown whether the instance was just created/saved or retrieved from the database.

Attachments (3)

decimal-field-default.patch (750 bytes ) - added by justin.driscoll@… 16 years ago.
Overrides the base "get_default" behavior for decimal fields
decimal-field-default.2.patch (747 bytes ) - added by justin.driscoll@… 16 years ago.
Overrides the base "get_default" behavior for decimal fields (fixed indent)
decimal_field_default_5903.patch (1.4 KB ) - added by Maciej Wiśniowski 16 years ago.
Patch that adds decimal.Decimal to smart_unicode

Download all attachments as: .zip

Change History (17)

by justin.driscoll@…, 16 years ago

Attachment: decimal-field-default.patch added

Overrides the base "get_default" behavior for decimal fields

by justin.driscoll@…, 16 years ago

Overrides the base "get_default" behavior for decimal fields (fixed indent)

comment:1 by Chris Beaven, 16 years ago

Needs tests: set
Triage Stage: UnreviewedDesign decision needed

I'll pass this through design decision -- seems like a fair enough request, yes?

by Maciej Wiśniowski, 16 years ago

Patch that adds decimal.Decimal to smart_unicode

comment:2 by Maciej Wiśniowski, 16 years ago

Cc: pigletto@… added
Needs tests: unset

I think the problem is in force_unicode not in DecimalField.
I've added a patch with tests for this.

This way it works just like other numeric fields, eg. float.

comment:3 by Maciej Wiśniowski, 16 years ago

Triage Stage: Design decision neededReady for checkin

comment:4 by Chris Beaven, 16 years ago

Triage Stage: Ready for checkinDesign decision needed

We'll leave as a design decision thanks pigletto.

comment:5 by Brian Rosner, 16 years ago

This seems to be rearing its head in more loudly recently. I have a default value of Decimal('0') on a models.DecimalField in a model and it is throwing a TypeError: a float is required in django.db.backends.util.format_number from BaseDatabaseOperations.value_to_db_decimal because the value is ran through force_unicode with strings_only=True which will do that type conversion on the Decimal object.

in reply to:  5 ; comment:6 by Karen Tracey <kmtracey@…>, 16 years ago

Replying to brosner:

What do you do to trigger the "float is required" type error? That seems highly inconsistent for a DecimalField since just yesterday I stumbled across the fact that floats are no longer allowed to be passed in as values to a DecimalField (see #8050). At any rate if it's new behavior it might be due to the same change as that one, but I don't think it's intended that floats be required anywhere here...only I can't recreate getting that exception for a field with a Decimal() default so I'm not sure how you are triggering it?

in reply to:  6 ; comment:7 by Brian Rosner, 16 years ago

Replying to Karen Tracey <kmtracey@gmail.com>:

Given this model:

class Cart(models.Model):
    desc = models.CharField(_("Description"), blank=True, null=True, max_length=10)
    date_time_created = models.DateTimeField(_("Creation Date"),
        default=datetime.datetime.now)
    customer = models.ForeignKey(Customer, blank=True, null=True)
    
    cache_shipping_total = models.DecimalField(max_digits=12, decimal_places=2,
        default=Decimal("0"))
    stale_shipping_total = models.BooleanField(default=True)

When you do:

>>> cart = Cart()
>>> cart.save()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/Users/brian/Code/loony/<ipython console> in <module>()

/Users/brian/svn/django/trunk/django/db/models/base.py in save(self=<class 'loony.blog.models.Cart'> instance)
    272         control the saving process.
    273         """
--> 274         self.save_base()
        self.save_base = undefined
    275 
    276     save.alters_data = True

/Users/brian/svn/django/trunk/django/db/models/base.py in save_base(self=<class 'loony.blog.models.Cart'> instance, raw=False, cls=<class 'loony.blog.models.Cart'>)
    328         if not pk_set or not record_exists:
    329             if not pk_set:
--> 330                 values = [(f, f.get_db_prep_save(raw and getattr(self, f.attname) or f.pre_save(self, True))) for f in meta.local_fields if not isinstance(f, AutoField)]
        values = undefined
        f = <django.db.models.fields.DecimalField object at 0x10d5cd0>
        f.get_db_prep_save = <bound method DecimalField.get_db_prep_save of <django.db.models.fields.DecimalField object at 0x10d5cd0>>
        raw = False
        global getattr = undefined
        self = undefined
        f.attname = 'cache_shipping_total'
        f.pre_save = <bound method DecimalField.pre_save of <django.db.models.fields.DecimalField object at 0x10d5cd0>>
        global True = undefined
        meta.local_fields = [<django.db.models.fields.AutoField object at 0x10d5d90>, <django.db.models.fields.DecimalField object at 0x10d5cd0>]
        global isinstance = undefined
        global AutoField = <class 'django.db.models.fields.AutoField'>
    331             else:
    332                 values = [(f, f.get_db_prep_save(raw and getattr(self, f.attname) or f.pre_save(self, True))) for f in meta.local_fields]

/Users/brian/svn/django/trunk/django/db/models/fields/__init__.py in get_db_prep_save(self=<django.db.models.fields.DecimalField object at 0x10d5cd0>, value=u'0')
    230     def get_db_prep_save(self, value):
    231         "Returns field's value prepared for saving into a database."
--> 232         return self.get_db_prep_value(value)
        self.get_db_prep_value = <bound method DecimalField.get_db_prep_value of <django.db.models.fields.DecimalField object at 0x10d5cd0>>
        value = u'0'
    233 
    234     def get_db_prep_lookup(self, lookup_type, value):

/Users/brian/svn/django/trunk/django/db/models/fields/__init__.py in get_db_prep_value(self=<django.db.models.fields.DecimalField object at 0x10d5cd0>, value=u'0')
    734     def get_db_prep_value(self, value):
    735         return connection.ops.value_to_db_decimal(value, self.max_digits,
--> 736                                                   self.decimal_places)
        self.decimal_places = 2
    737 
    738     def get_manipulator_field_objs(self):

/Users/brian/svn/django/trunk/django/db/backends/__init__.py in value_to_db_decimal(self=<django.db.backends.sqlite3.base.DatabaseOperations object at 0x70d5b0>, value=u'0', max_digits=12, decimal_places=2)
    300         if value is None:
    301             return None
--> 302         return util.format_number(value, max_digits, decimal_places)
        global util.format_number = <function format_number at 0x702b70>
        value = u'0'
        max_digits = 12
        decimal_places = 2
    303 
    304     def year_lookup_bounds(self, value):

/Users/brian/svn/django/trunk/django/db/backends/util.py in format_number(value=u'0', max_digits=12, decimal_places=2)
    122     """
    123     Formats a number into a string with the requisite number of digits and
    124     decimal places.
    125     """
--> 126     return u"%.*f" % (decimal_places, value)
        decimal_places = 2
        value = u'0'

TypeError: a float is required

in reply to:  7 comment:8 by Karen Tracey <kmtracey@…>, 16 years ago

Replying to brosner:

> /Users/brian/svn/django/trunk/django/db/models/fields/__init__.py in get_db_prep_value(self=<django.db.models.fields.DecimalField object at 0x10d5cd0>, value=u'0')
>     734     def get_db_prep_value(self, value):
>     735         return connection.ops.value_to_db_decimal(value, self.max_digits,
> --> 736                                                   self.decimal_places)
>         self.decimal_places = 2
>     737 

This is old code. Try updating to r8143 or later and see if that fixes it? I can't recreate the error on the current level. (I do still see the behavior in the original ticket description, though.)

comment:9 by Brian Rosner, 16 years ago

Argh, ok, thanks Karen. You are right. I *really* need to go back to my old method of tracking trunk. My local version of Django was one revision behind :P Updating to trunk does indeed fix this problem I am having. FTR, this ticket does solve the problem I had, but it appears it was tackled in a different way.

comment:10 by Antti Kaihola, 16 years ago

AFAICT this currently applies not only to default values of DecimalFields but explicitly initialized string values as well:

>>> c = Cart(cache_shipping_total='2.50')
>>> c.cache_shipping_total
'2.50'
>>> c.save()
>>> c.cache_shipping_total
'2.50'
>>> c2 = Cart.objects.get(pk=c.pk)
>>> c2.cache_shipping_total
Decimal("2.50")

Also, the string value is not converted to a Decimal object on save() but only when re-querying the object from the database. Neither of the suggested patches fixes this behaviour.

Should I open a new ticket or update the summary of this one?

in reply to:  10 comment:11 by Antti Kaihola, 16 years ago

Jacob replied to akaihola on #django-dev:

that's been discussed ad-nauseum in the past. models don't
have __setattr__ hooks for speed, so you can say
"model.intfield = "Fish"" for all Django cares.

![...] it's a well-known problem, but one without a "good" fix.

I think model validation will probably help out here; I'd
imagine that a side-effect of validating a model would coerce
data to the right types.

comment:12 by Brian Rosner, 15 years ago

Owner: changed from nobody to Brian Rosner
Status: newassigned
Triage Stage: Design decision neededAccepted

comment:14 by Brian Rosner, 15 years ago

Resolution: fixed
Status: assignedclosed

(In [9823]) Fixed #5903 -- DecimalField.get_default() now correctly returns a Decimal object when the model instance was not retrieved from the database. Thanks Justin Driscoll and pigletto.

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