Code

Opened 3 years ago

Closed 3 years ago

#16536 closed Bug (fixed)

inspectdb with numerical column names

Reported by: danodonovan Owned by: teraom
Component: Core (Management commands) Version: master
Severity: Normal Keywords: inspectdb
Cc: dan.odonovan@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

inspectdb produces model fields with numerical values for fieldnames when running with a (MySQL at least) DB with numerical column labels. For example, sql with column names 1 and 2 results in invalid python code:

    1 = models.CharField()
    2 = models.CharField()

I would submit a patch, but I don't want to suggest a default prefix name for these fields without advice. Would suggest 'number_%d' % db_column or similar.

Attachments (2)

inspectdb_numeric_fields_16536.diff (884 bytes) - added by teraom 3 years ago.
inspectdb_numeric_fields_16536_2.diff (963 bytes) - added by danodonovan 3 years ago.
ammended patch

Download all attachments as: .zip

Change History (8)

comment:1 Changed 3 years ago by aaugustin

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

This problem is similar to the situation when a database column name is a Python reserved word (such as 'pass', 'class' or 'for'). In such cases, inspectdb appends '_field' to the column name.

'number_%d' sounds OK to me.

Could you also add a comment in the generated code: "'Field renamed because it wasn't a valid Python identifier."?

Patch appreciated :)

comment:2 Changed 3 years ago by teraom

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

Changed 3 years ago by teraom

comment:3 Changed 3 years ago by teraom

  1. Is the usage of isdigit() correct here? I assume there will be no period in the column name.
  2. Should I use unicode.isnumeric() instead?

comment:4 Changed 3 years ago by danodonovan

  • Has patch set

Thanks teraom! Unfortunately there is more trickyness: As the column name has changed, we should add

     extra_params['db_column'] = unicode(column_name)

Without that unicode casting, my test db wasn't recognising db_column as being valid (they returned nothing).

att_name isn't a digit in python world so maybe we should do:

    att_name = 'number_%d' % int(att_name)

Revised patch attached. Dan

Changed 3 years ago by danodonovan

ammended patch

comment:5 Changed 3 years ago by danodonovan

  • Cc dan.odonovan@… added
  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 3 years ago by mtredinnick

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

In [16641]:

Teach inspectdb to handle SQL column names that are digits.

There's no accounting for taste in the way some people name columns,
apparently. Create a column with a name of "1" and inspectdb will still
produce valid Python code now. Fixed #16536. Thanks tereaom and
danodonovan.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.