Opened 8 years ago

Closed 6 years ago

Last modified 5 years ago

#26819 closed Bug (fixed)

Using a ArrayField on Meta.unique_together throws "unhashable type: 'list'" on validate_unique method

Reported by: Gleber Diniz Owned by: Demur Nodia
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

It happens on a second save (when there is already data to be validated)

On this line: "if row_data in seen_data:", because row_data contains a list and seen_data is a set (row_data: (1, [1, 1]) / seen_data: set())

Example to reproduce:

models:

class Map(models.Model):
    name = models.CharField('name', max_length=128)


class MapSpot(models.Model):
    map = models.ForeignKey('body.Map', related_name='spots')
    position = ArrayField(models.IntegerField(), size=2)

    class Meta:
        unique_together = [('map', 'position')]

admin:

class MapSpotInline(admin.TabularInline):
    model = MapSpot
    extra = 0


@admin.register(Map)
class MapAdmin(admin.ModelAdmin):
    inlines = [MapSpotInline]

Change History (19)

comment:1 by Tim Graham, 8 years ago

Component: Uncategorizedcontrib.postgres
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 by PREMANAND, 8 years ago

Owner: changed from nobody to PREMANAND
Status: newassigned

comment:9 by PREMANAND, 8 years ago

This issue occurs as the array item is stored in the lists and there is a mis-match while comparison.

The arrayitem is stored in row_data as list and list are not hashable as the error says.

if row_data in seen_data:

This issue can be fixed by changing this line ie; return a tuple instead of list.

https://github.com/PREM1980/django/blob/master/django/contrib/postgres/forms/array.py#L56

Please review and provide your comments.

comment:10 by Tim Graham, 8 years ago

Not sure without looking into the details myself, but it might be better if the validate_unique() method took care of casting as needed since changing the return value of to_python() could be backwards-incompatible.

comment:11 by PREMANAND, 8 years ago

Description: modified (diff)

As you mentioned, implemented fix in validate_unique() method. The above code fixes the issue and all the unit tests got passed.

The only problem is Im not sure how to write the test case. It needs to be simulated as if a user creates a form in the html page. Do you have any pointers for testing similar condition?

row_data = tuple(d._get_pk_val() if hasattr(d, '_get_pk_val') else tuple(d)
                                  if type(d) is list else d for d in row_data)
Last edited 8 years ago by PREMANAND (previous) (diff)

comment:12 by Tim Graham, 8 years ago

Take a look at tests/postgres_tests/test_array.py for the existing tests for ArrayField.

By the way, you inadvertently deleted the ticket's description in your last comment. Can you restore it? Deleting your comment should undo that action if you can't find another way.

comment:14 by Claude Paroz, 8 years ago

Description: modified (diff)

Description restored.

comment:15 by Meiyer, 8 years ago

This happens only for Inline admin models (only validate_unique() of BaseModelFormSet contains the problematic code).

This is not related to the database and is purely a Python issue. This is not limited to ArrayField; any custom field may be affected. In our case, the phonenumber_field.modelfields.PhoneNumberField is causing the error unhashable type: 'PhoneNumber'. Since Django documentation does not mandate that custom fields are hashable (i.e., define the __hash__() method), I suppose that most of the existing non-builtin fields are affected.

Looks like the solution is either to keep seen_data as a set() and rely on some property of the field (unreliable and error-prone, see sample code below), or give up set() and manually implement removal of duplicates.

    row_data = tuple(d._get_pk_val() if hasattr(d, '_get_pk_val') else 
                     d if d.__hash__ is not None else
                     d.__repr__()
                     for d in row_data)
Last edited 8 years ago by Meiyer (previous) (diff)

comment:17 by Tim Graham, 8 years ago

Component: contrib.postgresForms
Description: modified (diff)

I believe it affects all inline formsets, not just those in the admin. There's a PR without tests.

comment:18 by Mariusz Felisiak, 7 years ago

Owner: changed from PREMANAND to Mariusz Felisiak
Version: 1.9master

comment:19 by Mariusz Felisiak, 6 years ago

Owner: Mariusz Felisiak removed
Status: assignednew

comment:20 by Demur Nodia, 6 years ago

Owner: set to Demur Nodia
Status: newassigned

comment:22 by Demur Nodia, 6 years ago

Has patch: set

comment:23 by Carlton Gibson, 6 years ago

Patch needs improvement: set

Patch looks good; there were just a few adjustments needed (Review on PR).

Please uncheck Patch needs improvement when they're addressed and we'll get this in.
Thanks Demur!

comment:24 by Demur Nodia, 6 years ago

Patch needs improvement: unset

comment:25 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 06a11ef:

Fixed #26819 -- Fixed BaseModelFormSet.validate_unique() "unhashable type: list" crash.

comment:26 by Ivan Virabyan, 5 years ago

Although this has been fixed specifically for ArrayField, we still get this error with JSONField. Probably more generic solution needed.

comment:27 by Carlton Gibson, 5 years ago

Ivan, please open a new ticket with the minimal reproduce example. Thanks.

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