#34944: Missing or misinferred attributes in output fields of generated fields
-------------------------------------+-------------------------------------
     Reporter:  Paolo Melchiorre     |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  5.0
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:
     Keywords:  field, database,     |             Triage Stage:
  generated, output_field            |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Paolo Melchiorre:

Old description:

> Continuing with my experiments with the generated fields I found these
> two error situations which I reported together because they are very
> connected and could lead to a single solution.
>
> I do not rule out that there are other examples of use of the generated
> fields that could lead to the same errors and if they come to mind,
> please add them in the comments.
>
> == Misinferred attributes
>
> In the common example of a field generated as a concatenation of two
> CharField in a Model:
>
> {{{
> #!python
> class Person(models.Model):
>     first_name = models.CharField(max_length=255)
>     last_name = models.CharField(max_length=255)
>     full_name = models.GeneratedField(
>         expression=Concat(
>             "first_name", models.Value(" "), "last_name"
>         ),
>         db_persist=True,
>     )
> }}}
>
> The SQL code for the generated column has an automatically inferred max
> length of only 255 characters (`varchar(255)`), while the field should be
> able to contain strings of 511 characters (`varchar(511)`):
>
> {{{
> #!sql
> CREATE TABLE "community_person" (
>     "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT,
>     "first_name" varchar(255) NOT NULL,
>     "last_name" varchar(255) NOT NULL,
>     "full_name" varchar(255) GENERATED ALWAYS AS (
>         COALESCE("first_name", '') ||
>         COALESCE(COALESCE(' ', '') ||
>         COALESCE("last_name", ''), '')
>     ) STORED
> );
> }}}
>
> ==== Proposal
>
> To solve the problem you could alternatively:
>
> a. make the maximum length extraction process smarter for the
> automatically created output fields
> b. mandatorily require specifying the output field when some of the
> fields involved could lead to situations like the ones above
>

> == Missing attributes
>
> If in the previous example, I explicitly specify the output field without
> attributes, the migration is generated without generating errors:
>
> {{{
> #!python
> class Person(models.Model):
>     first_name = models.CharField(max_length=255)
>     last_name = models.CharField(max_length=255)
>     full_name = models.GeneratedField(
>         expression=Concat(
>             "first_name", models.Value(" "), "last_name"
>         ),
>         db_persist=True,
>         output_field=models.CharField(),
>     )
> }}}
>
> The SQL code contains an incorrect `varchar(None)` type:
>
> {{{
> #!sql
> CREATE TABLE "community_person" (
>     "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT,
>     "first_name" varchar(255) NOT NULL,
>     "last_name" varchar(255) NOT NULL,
>     "full_name" varchar(None) GENERATED ALWAYS AS (
>         COALESCE("first_name", '') ||
>         COALESCE(COALESCE(' ', '') ||
>         COALESCE("last_name", ''), '')
>     ) STORED
> );
> }}}
>
> which will generate an error when trying to apply the migration to the
> database:
>
> {{{
> #!console
> sqlite3.OperationalError: near "None": syntax error
> }}}
>

> ==== Proposal
>
> To solve the problem you could alternatively:
>
> a. make the SQL code generation process smarter so as to generate working
> SQL code
> b. request to specify mandatory attributes for the output fields raising
> an error in the migration creation phase
>
> == Cumulative proposal
>
> To solve both problems shown above, given the little time remaining
> before the release of the stable version of Django 5, the two most
> drastic solutions from both of the above proposals could be adopted.
>
> ==== Required output field
>
> Always require specifying the output field ''(except when you are sure
> that the extracted type cannot generate error situations?)''
>
> Example of error message:
> {{{
> #!console
> django.core.exceptions.FieldError: Expression doesn't contain an explicit
> type. You must set output_field.
> }}}
>
> ==== Required attributes
> Request to specify mandatory attributes for the output fields raising an
> error in the migration creation phase.
>
> {{{
> #!console
> $ python3 -m manage makemigrations
> SystemCheckError: System check identified some issues:
>
> ERRORS:
> community.Person.full_name: (fields.E120) CharFields must define a
> 'max_length' attribute.
> }}}

New description:

 Continuing with my experiments with the generated fields I found these two
 error situations which I reported together because they are very connected
 and could lead to a single solution.

 I do not rule out that there are other examples of use of the generated
 fields that could lead to the same errors and if they come to mind, please
 add them in the comments.

 === Misinferred attributes

 In the common example of a field generated as a concatenation of two
 CharField in a Model:

 {{{
 #!python
 class Person(models.Model):
     first_name = models.CharField(max_length=255)
     last_name = models.CharField(max_length=255)
     full_name = models.GeneratedField(
         expression=Concat(
             "first_name", models.Value(" "), "last_name"
         ),
         db_persist=True,
     )
 }}}

 The SQL code for the generated column has an automatically inferred max
 length of only 255 characters (`varchar(255)`), while the field should be
 able to contain strings of 511 characters (`varchar(511)`):

 {{{
 #!sql
 CREATE TABLE "community_person" (
     "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT,
     "first_name" varchar(255) NOT NULL,
     "last_name" varchar(255) NOT NULL,
     "full_name" varchar(255) GENERATED ALWAYS AS (
         COALESCE("first_name", '') ||
         COALESCE(COALESCE(' ', '') ||
         COALESCE("last_name", ''), '')
     ) STORED
 );
 }}}

 ==== Proposal

 To solve the problem you could alternatively:

 a. make the maximum length extraction process smarter for the
 automatically created output fields
 b. mandatorily require specifying the output field when some of the fields
 involved could lead to situations like the ones above


 === Missing attributes

 If in the previous example, I explicitly specify the output field without
 attributes, the migration is generated without generating errors:

 {{{
 #!python
 class Person(models.Model):
     first_name = models.CharField(max_length=255)
     last_name = models.CharField(max_length=255)
     full_name = models.GeneratedField(
         expression=Concat(
             "first_name", models.Value(" "), "last_name"
         ),
         db_persist=True,
         output_field=models.CharField(),
     )
 }}}

 The SQL code contains an incorrect `varchar(None)` type:

 {{{
 #!sql
 CREATE TABLE "community_person" (
     "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT,
     "first_name" varchar(255) NOT NULL,
     "last_name" varchar(255) NOT NULL,
     "full_name" varchar(None) GENERATED ALWAYS AS (
         COALESCE("first_name", '') ||
         COALESCE(COALESCE(' ', '') ||
         COALESCE("last_name", ''), '')
     ) STORED
 );
 }}}

 which will generate an error when trying to apply the migration to the
 database:

 {{{
 #!console
 sqlite3.OperationalError: near "None": syntax error
 }}}


 ==== Proposal

 To solve the problem you could alternatively:

 a. make the SQL code generation process smarter so as to generate working
 SQL code
 b. request to specify mandatory attributes for the output fields raising
 an error in the migration creation phase

 === Cumulative proposal

 To solve both problems shown above, given the little time remaining before
 the release of the stable version of Django 5, the two most drastic
 solutions from both of the above proposals could be adopted.

 ==== Required output field

 Always require specifying the output field ''(except when you are sure
 that the extracted type cannot generate error situations?)''

 Example of error message:
 {{{
 #!console
 django.core.exceptions.FieldError: Expression doesn't contain an explicit
 type. You must set output_field.
 }}}

 ==== Required attributes
 Request to specify mandatory attributes for the output fields raising an
 error in the migration creation phase.

 {{{
 #!console
 $ python3 -m manage makemigrations
 SystemCheckError: System check identified some issues:

 ERRORS:
 community.Person.full_name: (fields.E120) CharFields must define a
 'max_length' attribute.
 }}}

--

-- 
Ticket URL: <https://code.djangoproject.com/ticket/34944#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018b9f09c4be-7f892e9b-d457-4d25-b1c9-2ce91c397d5e-000000%40eu-central-1.amazonses.com.

Reply via email to