Opened 7 years ago

Closed 4 years ago

#12460 closed Bug (fixed)

inspectdb and field names ending with underscores

Reported by: 3gun <mihail.lukin@…> Owned by: kgibula
Component: Core (Management commands) Version: master
Severity: Normal Keywords: inspectdb underscore characters digits
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi,

There is a field named 'type_' in mysql table. Inspectdb inspects it as 'type_' and model does not validate. I suggest removing underscores from model field name and specifying column_db argument in field constructor.

< type_ = models.CharField(max_length=90, db_column='type_')

type = models.CharField(max_length=90, db_column='type_')

Sorry for posting without login. I've forgot my password and didn't find restoring feature on this site.

Attachments (4)

inspect_db_py_names.patch (3.0 KB) - added by Elijah Rutschman 6 years ago.
Patch that addresses trailing underscore issue as well as several other introspection field naming scenarios
inspectdb.diff (6.2 KB) - added by kgibula 5 years ago.
updated patch
12460-3.diff (8.6 KB) - added by Claude Paroz 5 years ago.
Updated patch
12460-4.diff (8.8 KB) - added by Claude Paroz 4 years ago.
Patch updated to current trunk

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by Russell Keith-Magee

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

I've confirmed that the problem exists, but your proposed solution won't work (at least, not in the general case). type_ is the name of a Python builtin, so you shouldn't name a model field type. An alternate approach would be to append or prepend a known character (e.g., xtype, type_1) to create a field name.

comment:2 Changed 7 years ago by Alex Gaynor

Django already handles the case of a python keyword: http://code.djangoproject.com/browser/django/trunk/django/core/management/commands/inspectdb.py#L64 . Having something with the same name as a builtin shouldn't matter.

comment:3 Changed 6 years ago by Elijah Rutschman

Keywords: characters digits added

There are a few other scenarios where invalid field names could be generated via the inspectdb command. For instance, a column name of "Foo/Bar", while unconventional, is a valid column name, at least in MySQL 5, but the invalid field name of 'foo/bar' would be generated.

Currently, inspectdb ensures that:

  • field names do not contain spaces or dashes
  • field names are not Python keywords

The patch I just attached improves this by also ensuring that:

  • field names do not contain any characters outside of a-zA-Z0-9_
  • field names do not contain double underscores
  • field names neither start nor end with underscores
  • field names do not start with digits

Changed 6 years ago by Elijah Rutschman

Attachment: inspect_db_py_names.patch added

Patch that addresses trailing underscore issue as well as several other introspection field naming scenarios

comment:4 Changed 6 years ago by Elijah Rutschman

Has patch: set

comment:5 Changed 6 years ago by Ramiro Morales

Needs tests: set

comment:6 Changed 6 years ago by Gabriel Hurley

Component: django-admin.py inspectdbCore (Management commands)

comment:7 Changed 5 years ago by Matt McClanahan

Severity: Normal
Type: Bug

comment:8 Changed 5 years ago by kgibula

Owner: changed from nobody to kgibula
Status: newassigned

Patch can't be applied to trunk anymore. It would also convert both ''Foo__'' and ''foo_'' (valid MySQL column names) to ''foo_field''.

Last edited 5 years ago by kgibula (previous) (diff)

Changed 5 years ago by kgibula

Attachment: inspectdb.diff added

updated patch

comment:9 Changed 5 years ago by kgibula

Easy pickings: unset
Patch needs improvement: set

I tested it on MySQL, PostgreSQL and SQLite. Currently tests pass only on MySQL and PostgreSQL. When db_column keyword argument is added, on MySQL and PostgreQSL it looks like this: db_column='name_of_field'. On SQLite however it looks like this: db_column=u'name_of_field'. I am not sure which one is correct.

Changed 5 years ago by Claude Paroz

Attachment: 12460-3.diff added

Updated patch

comment:10 Changed 5 years ago by Claude Paroz

Needs tests: unset
Patch needs improvement: unset
UI/UX: unset
Version: 1.1SVN

Here's an updated patch where I have slightly refactored things to isolate the normalization process of the field name. I purposely did not include the non-ascii characters issue which was also addressed by previous "inspectdb.diff" patch. If the committer finds it's better to also include it, I can complete the patch. Otherwise, I'll gladly add a specific patch for that issue in #16737 after this one is committed.

Changed 4 years ago by Claude Paroz

Attachment: 12460-4.diff added

Patch updated to current trunk

comment:11 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: assignedclosed

In [395c6083af93cc37c7d16ef4db451091841cefdc]:

Fixed #12460 -- Improved inspectdb handling of special field names

Thanks mihail lukin for the report and elijahr and kgibula for their
work on the patch.

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