Opened 6 years ago

Closed 3 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 elijahr 5 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 4 years ago.
updated patch
12460-3.diff (8.6 KB) - added by claudep 4 years ago.
Updated patch
12460-4.diff (8.8 KB) - added by claudep 3 years ago.
Patch updated to current trunk

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 6 years ago by Alex

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 5 years ago by elijahr

  • 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 5 years ago by elijahr

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

comment:4 Changed 5 years ago by elijahr

  • Has patch set

comment:5 Changed 5 years ago by ramiro

  • Needs tests set

comment:6 Changed 4 years ago by gabrielhurley

  • Component changed from django-admin.py inspectdb to Core (Management commands)

comment:7 Changed 4 years ago by mattmcc

  • Severity set to Normal
  • Type set to Bug

comment:8 Changed 4 years ago by kgibula

  • Owner changed from nobody to kgibula
  • Status changed from new to assigned

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 4 years ago by kgibula (previous) (diff)

Changed 4 years ago by kgibula

updated patch

comment:9 Changed 4 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 4 years ago by claudep

Updated patch

comment:10 Changed 4 years ago by claudep

  • Needs tests unset
  • Patch needs improvement unset
  • UI/UX unset
  • Version changed from 1.1 to SVN

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 3 years ago by claudep

Patch updated to current trunk

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

  • Resolution set to fixed
  • Status changed from assigned to closed

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