unique_together bug when it's not a tuple. Validation missing

The current patch uses type(x) == ... when it should be using isinstance(x, (list, tuple)). New error wording could be polished a bit too.

Pushing to design decision to see if the idea in general is deemed worthy.

Personally, if we're going to type-check the value I think we should take the opposite approach -- use if isinstance(val, basestring) to only catch things that are strings. Limiting to a tuple or a list, while it fits with common practice, does limit the types of iterable things people could use in unique_together, and I wouldn't want to do that needlessly.

Hi ubernostrum, I think you've got a good point of view. However almost every setting in Django that needs more than one value, use tuples or lists; if all should be a string, how do you transform unique_together = (('field1', 'field2'),)? ex: unique_together="field1, field2" or unique_together="'field1, field2' 'field1, field3'"

I mean, I think it's hard (not impossible) not use lists or tuples in those situations and also IMHO that's "Design decision".

I agree with James, the error checking is backwards here (and the above comment misunderstand the point James is making I suspect). Do a check like

if isinstance(val, basestring):
    raise TypeError, '....'

and then no further indenting is needed for the common case. It wasn't immediately clear from the bug description, but I gather you are just trying to work around the case of when people use a string when a tuple is required. So test for that one case, instead of ruling out iterators that are permissible.

If it's a simple check like this, catching a common error case, we can include it.

It just needed a very little change :-)

Also, please change the error message. I'm not a English native speaker but I think you have noticed this :-)

Is this ticket going to be added to the trunk?

Closing as invalid since existing validation will error on the case that the field name doesn't exist. In others words, the following already fails validation...

unique_together = 'foo, bar'

or

unique_together = ('foo', 'bar')

so why should we test for the single case of unique_together being a string?

However, we should probably allow the single tuple, rather than the tuple of tuples for convenience. I've opened #5460 for this new issue.