Opened 14 years ago

Closed 10 years ago

#13711 closed Bug (fixed)

Very long field names on models cause problems

Reported by: Russell Keith-Magee Owned by: ANUBHAV JOSHI
Component: Database layer (models, ORM) Version: dev
Severity: Release blocker Keywords:
Cc: Shai Berger Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Under MySQL, you get

_mysql_exceptions.OperationalError: (1059, "Identifier name 'verylongmodelnamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz_id' is too long")

raised as an error during runtime.

Under Postgres, you get

NOTICE:  identifier "verylongmodelnamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz_id" will be truncated to "verylongmodelnamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"

appear silently in the logs.

This problem (and the solution) is analogous to #13528 for db_table.

Change History (26)

comment:1 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Russell Keith-Magee, 14 years ago

For the record - the patch for #8901 in [13328] contains a model that reveals this problem for MySQL. Removing the anti-MySQL guard for that test would be one part of the fix for this problem.

comment:3 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

comment:4 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:5 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:6 by vernondcole@…, 11 years ago

This behavior is also seen in MS SQL Server, so MySQL is not alone in limiting the length of table names.

Furthermore, the test workaround in regressiontests/backends/models.py saying:

if connection.features.supports_long_model_names:

only works if the _default_ database does not support long names. For example, if 'default' is SQLite and 'other' is MySQL, the testing will blow up.

comment:7 by ANUBHAV JOSHI, 10 years ago

Owner: changed from nobody to ANUBHAV JOSHI
Status: newassigned

comment:9 by ANUBHAV JOSHI, 10 years ago

comment:10 by ANUBHAV JOSHI, 10 years ago

Has patch: set

comment:11 by ANUBHAV JOSHI, 10 years ago

Also when db_table name truncation is done, we get a weird message:

#models.py
class aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(models.Model):

name = models.CharField(max_length=40)

python manage.py syncdb

Creating table probs_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa36a3
Warning: Data truncated for column 'name' at row 13

Instead of reporting about the truncated name of the model table, it seems to tell about the first field of that table.
This is something done by the backend I believe.(Correct me if I am wrong.)
Assuming if above is correct, then what we can do is create a warning using warning.warn(...) about the correct issue.
Then we can remove the false warning arising from the db.

comment:12 by ANUBHAV JOSHI, 10 years ago

I hope to work on this in GSoC 2014

comment:13 by ANUBHAV JOSHI, 10 years ago

Version: 1.2master

New PR: https://github.com/django/django/pull/2738

Check and test added.

Last edited 10 years ago by ANUBHAV JOSHI (previous) (diff)

comment:14 by Tim Graham, 10 years ago

Patch needs improvement: set

We are going to revisit this after working on #13528. I think we might be able to same a similar approach here.

comment:15 by ANUBHAV JOSHI, 10 years ago

As commented on #13528, the problem seems much bigger. So perhaps we could get rid of this bug with my patch keeping in mind that this bug affects both single-db and multi-db users.
My patch solves the problem for both cases and I think user should be making Model keeping in mind the database(s) he is going to use and the patch does the same thing.

comment:16 by Tim Graham, 10 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:17 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 91f1b6dcdc5da47d7794a55e3114820407a5bd62:

Fixed #13711 -- Model check added to ensure that auto-generated column name is within limits of the database.

Thanks russellm for report and Tim Graham for review.

comment:18 by Tim Graham, 10 years ago

Cc: Shai Berger added
Has patch: unset
Severity: NormalRelease blocker
Triage Stage: Ready for checkinAccepted

Reopening as these changes prevent the Oracle tests from running:

ERRORS:
backends.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ: (models.E018) Autogenerated column name too long for field "charfield_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz". Maximum length is "30" for database "default".
	HINT: Set the column name manually using 'db_column'.
backends.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ: (models.E018) Autogenerated column name too long for field "primary_key_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz". Maximum length is "30" for database "default".
	HINT: Set the column name manually using 'db_column'.
backends.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ: (models.E019) Autogenerated column name too long for M2M field "verylongmodelnamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz_id". Maximum length is "30" for database "default".
	HINT: Use 'through' to create a separate model for M2M and then set column_name using 'db_column'.
sites_framework.CustomArticle: (models.E018) Autogenerated column name too long for field "places_this_article_should_appear_id". Maximum length is "30" for database "default".
	HINT: Set the column name manually using 'db_column'.

comment:19 by Tim Graham, 10 years ago

As discussed with Shai on IRC, column names are auto-truncated in Oracle so the model check isn't needed so much there, or at least the errors should be reduced to warnings. 3rd-party backends may implemented similar behavior so we should keep that in mind when deciding on a solution.

comment:20 by ANUBHAV JOSHI, 10 years ago

One approach:

Adding a attribute which tells whether truncate_name is called or not, like connection.supports_auto_truncation and then running the model check only for those dbs which have this set to False.

EDITS:
This could be similar to the one which was removed:connection.supports_long_model_names(This was used for the dbs which either did truncation calling truncate_name or by itself by skipping the chars after its max limit. This second part created problems). It would be like renaming this attribute to the above and correcting its usage because the earlier use created problems and the new usage for checking only usage of truncate_name does not.
This should be True for any backends that uses truncate_name.

Last edited 10 years ago by ANUBHAV JOSHI (previous) (diff)

comment:21 by ANUBHAV JOSHI, 10 years ago

If we can make a decision then I will make a patch for this.

comment:22 by ANUBHAV JOSHI, 10 years ago

I have made a patch for what I suggested: https://github.com/coder9042/django/compare/gsoc_13711
IMO this is the right way for this issue and also now the check runs for all backends rather than only for the one with minimum allowed length. Also the backends with the connection.feature.supports_auto_truncation set to True are skipped.

comment:23 by Tim Graham, 10 years ago

The feature flag makes sense to me, but I think a better name would be something like truncates_column_names.

Why does it now make sense to check all backends rather than only the one with the minimum length? As far as I can tell, the user will only care about the error for the database with the minimum length.

comment:24 by Shai Berger, 10 years ago

Resolution: fixed
Status: closednew

Sorry for not being able to respond earlier.

The feature flag certainly makes sense, and could be used elsewhere; but I don't think it is the right approach for solving this issue. Its name should probably be truncates_names -- @timo is in the right direction, but it isn't just column names that get truncated, other names (tables, indexes) do too.

Oracle has specific related problems: Very long ago, auto-generated M2M table name truncation was implemented in a way that is different from truncation in the other cases (user-specified or auto-generated names for explicitly defined tables and columns). This is usually not a problem, but in some cases it is (it was a problem for South, and I'm not quite sure what the new migrations do about it). Mind you, it is not a problem in Oracle itself, but in the Django Oracle backend, but it cannot be corrected without breaking backwards-compatibility.

Such an issue, when the names are long enough to trigger it, warrants a warning IMO. But a flag "truncates_m2m_table_names_differently" makes no sense, so I don't see room for including such a check in the backend-agnostic core. It belongs in the backend itself.

While at it, we could add other checks about names (sometimes, Oracle sees a quoted keyword as a keyword, not a name, and fails things for no good reason). It's not just the lengths.

While @timo is right about name length checks (checking the minimum is enough), this is not so for these other checks.

I'm not familiar enough with the checks framework -- how hard would it be to define backend-speicific checks in the backend?

If the checks can be defined in the backends, I think it might be better to include all name checks -- length and otherwise -- in the backend, sharing code using the backends' inheritance trees.

I can see an argument that I am conflating length checks, which can be generic, with other checks, but I think it is better to have name checks united.

comment:25 by ANUBHAV JOSHI, 10 years ago

I think that this ticket is about solving issue in long name truncation and adding a relevant flag does that.
What @shai says is a much bigger issue which must be dealt separately in a different ticket where we can discuss for available approaches.

comment:26 by ANUBHAV JOSHI, 10 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top