[Django] #26120: HStoreField update error when values of different types
Django
----------------------------------+-----------------
Reporter: CGenie | Owner:
Type: Uncategorized | Status: new
Component: contrib.postgres | Version: 1.9
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+-----------------
Suppose I have a simple model:
{{{
class Simple(models.Model):
data = HStoreField(default={})
}}}
I can save hstore with mixed types, which is normal:
{{{
simple = Simple.objects.create(data = {'a': 1, 'b': 'B'})
}}}
But the `update` function throws an exception:
{{{
Simple.objects.filter(id=simple.id).update(data={'a': 1, 'b': 'B'})
}}}
{{{
Traceback (most recent call last):
File "failing.py", line 19, in <module>
show_error()
File "failing.py", line 11, in show_error
simple = Simple.objects.create(data={'a': 1, 'b': 'B'})
File "/usr/local/lib/python2.7/dist-
packages/django/db/models/manager.py", line 127, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py",
line 348, in create
obj.save(force_insert=True, using=self.db)
File "/usr/local/lib/python2.7/dist-packages/django/db/models/base.py",
line 734, in save
force_update=force_update, update_fields=update_fields)
File "/usr/local/lib/python2.7/dist-packages/django/db/models/base.py",
line 762, in save_base
updated = self._save_table(raw, cls, force_insert, force_update,
using, update_fields)
File "/usr/local/lib/python2.7/dist-packages/django/db/models/base.py",
line 846, in _save_table
result = self._do_insert(cls._base_manager, using, fields, update_pk,
raw)
File "/usr/local/lib/python2.7/dist-packages/django/db/models/base.py",
line 885, in _do_insert
using=using, raw=raw)
File "/usr/local/lib/python2.7/dist-
packages/django/db/models/manager.py", line 127, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py",
line 920, in _insert
return query.get_compiler(using=using).execute_sql(return_id)
File "/usr/local/lib/python2.7/dist-
packages/django/db/models/sql/compiler.py", line 974, in execute_sql
cursor.execute(sql, params)
File "/usr/local/lib/python2.7/dist-
packages/django/db/backends/utils.py", line 79, in execute
return super(CursorDebugWrapper, self).execute(sql, params)
File "/usr/local/lib/python2.7/dist-
packages/django/db/backends/utils.py", line 64, in execute
return self.cursor.execute(sql, params)
File "/usr/local/lib/python2.7/dist-packages/django/db/utils.py", line
98, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/usr/local/lib/python2.7/dist-
packages/django/db/backends/utils.py", line 64, in execute
return self.cursor.execute(sql, params)
DataError: invalid input syntax for integer: "B"
LINE 1: ...("data") VALUES (hstore(ARRAY['a', 'b'], ARRAY[1, 'B'])) RET...
}}}
See the toy project at https://github.com/CGenie/django_hstore_field_fail
This fails for 1.8 as well as for 1.9.
--
Ticket URL: <https://code.djangoproject.com/ticket/26120>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: CGenie | Owner:
Type: | Status: new
Cleanup/optimization |
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* severity: Release blocker => Normal
* cc: mjtamlyn (added)
* needs_better_patch: => 0
* needs_tests: => 0
* version: 1.9 => 1.8
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
Comment:
[https://docs.djangoproject.com/en/1.9/ref/contrib/postgres/fields/#hstorefield
The documentation] says, "A field for storing mappings of strings to
strings." I'm not sure if any change should be made, but doesn't qualify
as a release blocker in my opinion.
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:1>
Django
Reporter: CGenie | Owner:
Type: | Status: new
Cleanup/optimization |
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mjtamlyn):
Yes, it's not a release blocker. Interestingly, I didn't get the same
behaviour here, I got errors for any direct ORM usage with bad typing -
the Form layer will tidy it up for you, but the model layer doesn't seem
to.
However you do get more sensible behaviour from `CharField` for example
here. It's a pretty straightforwards fix I think, should just need to add
a `get_prep_value` method to the field.
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:2>
Django
Reporter: CGenie | Owner:
Type: | Status: new
Cleanup/optimization |
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
What should be done in that method? Casting all keys/values in the
dictionary to a string or raising an error for non-strings? The second
option seems safer to me in terms of hiding possible developer error.
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:3>
Django
Reporter: CGenie | Owner:
Type: | Status: new
Cleanup/optimization |
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mjtamlyn):
The former actually, that's what most fields do - You can pass `'1'` to an
`IntegerField` and `1` to a `CharField` and they'll cast it appropriately.
This casting is actually pretty useful for more complex fields often - you
can pass `'2015-01-01'` to a `DateField` rather than having to pass a
`datetime` instance. The developer error point is a fair one, but
consistency with the rest of the ORM is more important.
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: CGenie | Owner:
Type: Cleanup/optimization | Status: new
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:5>
Django
--------------------------------------+------------------------------------
Reporter: CGenie | Owner:
Type: Cleanup/optimization | Status: new
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by CGenie):
PostgreSQL supports arbitrary `hstore` values. Casting values to strings
is a bad idea imho. Since the `HStoreField` is added to expose PostgreSQL-
specific functionality, it should be supported fully, not partially with
hacks around Python code. Especially that setting values one by one works
fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: CGenie | Owner:
Type: Cleanup/optimization | Status: new
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
Replying to [comment:6 CGenie]:
> PostgreSQL supports arbitrary `hstore` values. Casting values to strings
is a bad idea imho. Since the `HStoreField` is added to expose PostgreSQL-
specific functionality, it should be supported fully, not partially with
hacks around Python code. Especially that setting values one by one works
fine.
PostgreSQL will cast values to `text`
[http://www.postgresql.org/docs/current/static/hstore.html#HSTORE anyway]:
> This module implements the hstore data type for storing sets of
key/value pairs within a single PostgreSQL value. This can be useful in
various scenarios, such as rows with many attributes that are rarely
examined, or semi-structured data. Keys and values are simply text
strings.
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: CGenie | Owner:
Type: Cleanup/optimization | Status: new
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by CGenie):
Ah, OK, sorry, my bad, didn't read thoroughly enough :P
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:8>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by gchp):
* status: new => assigned
* owner: => gchp
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:9>
Django
--------------------------------------+------------------------------------
Reporter: CGenie | Owner: gchp
Type: Cleanup/optimization | Status: assigned
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by gchp):
* has_patch: 0 => 1
Comment:
Added a [https://github.com/django/django/pull/6039 pull request] for this
issue. Let me know your comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:10>
Django
--------------------------------------+------------------------------------
Reporter: CGenie | Owner: gchp
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"8dea9f089dcb8aea2c417c91344c4e680431e972" 8dea9f0]:
{{{
#!CommitTicketReference repository=""
revision="8dea9f089dcb8aea2c417c91344c4e680431e972"
Fixed #26120 -- Made HStoreField cast keys and values to strings.
HStoreField now converts all keys and values to string before they're
saved to the database.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26120#comment:11>