varchar(255) converted into varchar(255) NOT NULL after inspectdb followed by migrate

[Paul Dejean]

show create table on the original database yields:

device_mac_ethernet varchar(255) DEFAULT NULL

inspectdb convert this into:

models.CharField(max_length=255, blank=True)

makemigrations followed by migrate converts that into:

device_mac_ethernet varchar(255) NOT NULL

[Paul Dejean]

If I had to guess this is a case of: hey "varchar(255) .* NULL" totally matches "varchar(255) NOT NULL"

Also I might submit a patch for this *fingers crossed*

[Claude Paroz]

The fact that inspectdb doesn't add null=True on CharFields/TextFields is based on a Django "good practice", explained in ​https://docs.djangoproject.com/en/stable/ref/models/fields/#null

Now the question is, should inspectdb reflect the current database structure as faithfully as possible, or should it entice users to follow good practices? At a minimum, Django should add a comment that it has not respected the NULL flag on the introspected field. Accepted the ticket on that base.

[Paul Dejean]

How about adding a command line option to inspectdb so the user can choose. Good practice being enforced can be the default but users who need to deviate for some reason can do so without having to hack the inspectdb source as I did.

[Claude Paroz]

inspectdb output has to be reviewed in all cases. For me, the choice is:

• keep the current output and add a comment like: # Field was detected as NULL, but we didn't add null=True, see https://docs.djangoproject.com/en/stable/ref/models/fields/#null

• change the output to let null=True and add the comment: # Read https://docs.djangoproject.com/en/stable/ref/models/fields/#null and consider removing null=True

I'm currently favoring the second option.

[Carl Meyer]

Given those two choices, I'm in favor of the second option as well, though I don't even think the comment is really necessary.

I think Django's advice re null CharFields is questionable, but even if we accept that advice, a CharField(null=True) is still a valid, functional configuration, and there is no reason for inspectdb to prohibit it and intentionally generate models that don't match the source database.

[Tim Graham]

I agree with Carl.

[Claude Paroz]

​https://github.com/django/django/pull/3395

[Claude Paroz <claude@…>]

In 4fe6824ffd319b199140e291f0f742e7fe1f4615:

Fixed #23679 -- Fixed null introspection for char/text fields

Thanks Paul Dejean for the report.