Opened 11 years ago

Closed 11 years ago

#22289 closed Bug (needsinfo)

Field with Validator always considered changed in migrations

Reported by: Daniel Hahler Owned by: nobody
Component: Migrations Version: 1.7-alpha-2
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am having the following model, and schemamigration --auto considers it to be changed always.

This appears to get triggered through the RegexValidator (commenting it, does not cause the model to be considered different every time).

from django.core.validators import RegexValidator
class Model(TimeStampedModel):
    foo_id = models.CharField(
        verbose_name='FOO ID',
        max_length=200,  # allow for URL in form, cleaned in clean_foo_id
        validators = [RegexValidator(r'(?i)^(?:(?:https?://)?www.foo.com/title/)?(tt\d+)(?:/.*)?$')],
        help_text=_('…'),
        blank=True,
        null=True,
        unique=True)

This _might_ be related to #22255, but probably only because it's about the same field.. ;)

Change History (10)

comment:1 by Sasha Romijn, 11 years ago

I don't think it is related to #22255 - but it is related to #21275.

comment:2 by Sasha Romijn, 11 years ago

Resolution: invalid
Status: newclosed

I don't think this is a bug in Django. In current master, there is no command schemamigration --auto, there is only makemigrations. When I test a fresh project with your model, using the new built-in schema migrations, I can not reproduce this issue - it consistently works correctly. Therefore, this may be a bug in South (which implements schemamigration --auto, but does not appear to be one in Django.

comment:3 by Daniel Hahler, 11 years ago

Resolution: invalid
Status: closednew

Sorry for the confusion: I was copying the commented line from the Makefile.

I am not using South anymore (removed from the virtualenv also), and use manage.py makemigrations app.

Good to know that you cannot reproduce it though. I will see if I can come up with a test case in the next days.

comment:4 by Daniel Hahler, 11 years ago

@erikr: have you tested it using PostgreSQL? (I am using it)

comment:5 by Daniel Hahler, 11 years ago

The problem is that the deconstructor (<bound method CharField.deconstruct of <django.db.models.fields.CharField: imdb_id>>) returns different values/objects for the validators:

ipdb> old_field_dec
(u'django.db.models.CharField', [], {u'null': True, u'validators': [<django.core.validators.RegexValidator object at 0x1cac790>], u'max_length': 200, u'blank': True, u'help_text': u'IMDB ID (starts with "tt") or IMDB URL.', u'unique': True, u'verbose_name': 'IMDB ID'})
ipdb> new_field_dec
(u'django.db.models.CharField', [], {u'null': True, u'validators': [<django.core.validators.RegexValidator object at 0x2cb5ad0>], u'max_length': 200, u'blank': True, u'help_text': <django.utils.functional.__proxy__ object at 0x2d872d0>, u'unique': True, u'verbose_name': 'IMDB ID'})
ipdb> l
    334             new_model_state = self.to_state.models[app_label, model_name]
    335             old_field_name = renamed_fields.get((app_label, model_name, field_name), field_name)
    336             old_field_dec = old_model_state.get_field_by_name(old_field_name).deconstruct()[1:]
    337             new_field_dec = new_model_state.get_field_by_name(field_name).deconstruct()[1:]
    338             if old_field_dec != new_field_dec:
4-> 339                 self.add_to_migration(
    340                     app_label,
    341                     operations.AlterField(
    342                         model_name=model_name,
    343                         name=field_name,
    344                         field=new_model_state.get_field_by_name(field_name),

I have noticed that there is AlterField.__eq__, which does never get called though?!

    def __eq__(self, other):
        return (
            (self.__class__ == other.__class__) and
            (self.name == other.name) and
            (self.model_name.lower() == other.model_name.lower()) and
            (self.field.deconstruct()[1:] == other.field.deconstruct()[1:])
        )

comment:6 by Sasha Romijn, 11 years ago

I used SQLite, not PostgreSQL, but I strongly doubt that this is a database-specific issue: validators aren't actually stored in the database, as far as I know.

Regarding comment:5: it makes sense that the instances might be two different ones, that's why there's a custom __eq__. The __eq__ you're looking at doesn't seem to be the right one, when comparing these two tuples, it should call __eq__ of RegexValidator. I notice that in there, it compares self.regex, which at that point is a compiled regex, not the original string. So the next step would be to look into whether, in your case, RegexValidator.__eq__ says they are not equal, and why.

comment:7 by Daniel Hahler, 11 years ago

Compiled patterns (_sre.SRE_Pattern) are considered to equal/identical for the same pattern (from ipyhon shell).

RegexValidator's __eq__ only tests for == for the SRE_Pattern, which fails in my case:

ipdb> v1.regex
<_sre.SRE_Pattern object at 0x1b0e470>
ipdb> _v2.regex
<_sre.SRE_Pattern object at 0x2abbdd0>
ipdb> _v1.regex == _v2.regex
False
ipdb> _v1.regex.
_v1.regex.findall     _v1.regex.flags       _v1.regex.groups      _v1.regex.pattern     _v1.regex.search      _v1.regex.sub         
_v1.regex.finditer    _v1.regex.groupindex  _v1.regex.match       _v1.regex.scanner     _v1.regex.split       _v1.regex.subn        
ipdb> _v1.regex.pattern
'(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$'
ipdb> _v2.regex.pattern     
'(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$'
ipdb> _v2.regex.flags
2
ipdb> _v1.regex.flags     
2
ipdb> pprint(_v1)
<django.core.validators.RegexValidator object at 0x1aa3350>
ipdb> pprint(_v1.__dict__)
{'_constructor_args': (('(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$',),
                       {}),
 'regex': <_sre.SRE_Pattern object at 0x1b0e470>}
ipdb> pprint(_v2.__dict__)                                                                                                                                                
{'_constructor_args': (('(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$',),
                       {}),
 'regex': <_sre.SRE_Pattern object at 0x2abbdd0>}

Maybe RegexValidator.__eq__ needs to get enhanced, to compare the relevant regex attributes?

I am using Python 2.7.5+ (Ubuntu Linux), on a 64bit.
Something must causing the SRE_Pattern objects to not get re-used in my case.

(full output of all attributes:
ipdb> [_v1.regex.__getattribute__(x) for x in ['findall', 'finditer', 'flags', 'groupindex', 'groups', 'match', 'pattern', 'scanner', 'search', 'split', 'sub', 'subn']]
[<built-in method findall of _sre.SRE_Pattern object at 0x1b0e470>, <built-in method finditer of _sre.SRE_Pattern object at 0x1b0e470>, 2, {}, 1, <built-in method match of _sre.SRE_Pattern object at 0x1b0e470>, '(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$', <built-in method scanner of _sre.SRE_Pattern object at 0x1b0e470>, <built-in method search of _sre.SRE_Pattern object at 0x1b0e470>, <built-in method split of _sre.SRE_Pattern object at 0x1b0e470>, <built-in method sub of _sre.SRE_Pattern object at 0x1b0e470>, <built-in method subn of _sre.SRE_Pattern object at 0x1b0e470>]

ipdb> [_v2.regex.__getattribute__(x) for x in ['findall', 'finditer', 'flags', 'groupindex', 'groups', 'match', 'pattern', 'scanner', 'search', 'split', 'sub', 'subn']]
[<built-in method findall of _sre.SRE_Pattern object at 0x2abbdd0>, <built-in method finditer of _sre.SRE_Pattern object at 0x2abbdd0>, 2, {}, 1, <built-in method match of _sre.SRE_Pattern object at 0x2abbdd0>, '(?i)^(?:(?:https?://)?www.imdb.com/title/)?(tt\\d+)(?:/.*)?$', <built-in method scanner of _sre.SRE_Pattern object at 0x2abbdd0>, <built-in method search of _sre.SRE_Pattern object at 0x2abbdd0>, <built-in method split of _sre.SRE_Pattern object at 0x2abbdd0>, <built-in method sub of _sre.SRE_Pattern object at 0x2abbdd0>, <built-in method subn of _sre.SRE_Pattern object at 0x2abbdd0>]
)

comment:8 by Daniel Hahler, 11 years ago

btw: the tests in tests/validators work fine, and they also compare RegexValidator instances.

comment:9 by Sasha Romijn, 11 years ago

Perhaps, but then I still find it suspicious that the tests work for you, and that it seems to work for me. Could you publish a small demo somewhere, which replicates the problem for you? Preferably with SQLite, assuming you can replicate the problem with that. Then I can see whether I can reproduce this myself somehow.

comment:10 by Baptiste Mispelon, 11 years ago

Resolution: needsinfo
Status: newclosed

I'm also unabled to reproduce this (tried sqlite and postgres).

I did make one change compared to the models you provided: I replaced TimeStampedModel with a regular models.Model.
So maybe the problem is in TimeStampedModel?

On a sidenote, compiled regexp don't implement __eq__ in python, so two different regexp objects will always be different, even if they use the same pattern and flags.

I'll close this as needsinfo. Can you reopen with either the full definition of TimestampedModel or a fully contained testcase (something that doesn't use external models)?

Thanks.

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