Re: [Django] #13711: Very long field names on models cause problems
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.2
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by vernondcole@…):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Component: Database layer | Status: assigned
(models, ORM) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by anubhav9042):
* owner: nobody => anubhav9042
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
(models, ORM) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
https://github.com/django/django/pull/2102
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
(models, ORM) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
https://github.com/django/django/pull/2102
Please see the new PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
(models, ORM) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
(models, ORM) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
(models, ORM) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
I hope to work on this in GSoC 2014
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 1.2 => master
Comment:
New PR: https://github.com/django/django/pull/2738
Check and test added for dbs that do not support long model names.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
(models, ORM) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
We are going to revisit this after working on #13528. I think we might be
able to same a similar approach here.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
(models, ORM) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: assigned
(models, ORM) | Version: master
Severity: Normal | Resolution:
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timo):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
(models, ORM) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"91f1b6dcdc5da47d7794a55e3114820407a5bd62"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: closed
(models, ORM) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timo):
* cc: shai (added)
* has_patch: 1 => 0
* severity: Normal => Release blocker
* stage: Ready for checkin => Accepted
Comment:
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'.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: closed
(models, ORM) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timo):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: closed
(models, ORM) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
One approach:
Adding a setting which tells whether `truncate_name` is called or not,
like `connection.support_auto_truncation` and then running the model check
only for those dbs which have this set to False.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: closed
(models, ORM) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
If we can make a decision then I will make a patch for this.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: closed
(models, ORM) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: closed
(models, ORM) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timo):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
(models, ORM) | Version: master
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
Component: Database layer | Status: new
(models, ORM) | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: russellm | Owner:
Type: Bug | anubhav9042
(models, ORM) | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In
[https://code.djangoproject.com/changeset/1c50d6ae2bee31ccf605066186d4d48aaf2de7ea
1c50d6ae2bee31ccf605066186d4d48aaf2de7ea]
--
Ticket URL: <https://code.djangoproject.com/ticket/13711#comment:26>