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 , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 14 years ago
comment:3 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:6 by , 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 , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:10 by , 11 years ago
Has patch: | set |
---|
comment:11 by , 11 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:13 by , 10 years ago
Version: | 1.2 → master |
---|
New PR: https://github.com/django/django/pull/2738
Check and test added for dbs that do not support long model names.
comment:14 by , 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 , 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 , 10 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:17 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:18 by , 10 years ago
Cc: | added |
---|---|
Has patch: | unset |
Severity: | Normal → Release blocker |
Triage Stage: | Ready for checkin → Accepted |
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 , 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 , 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
.
comment:22 by , 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 , 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 , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
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 , 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 , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.