[Django] #34861: KeyTextTransform incompatible with GeneratedField

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------
Reporter: Paolo | Owner: nobody
Melchiorre |
Type: Bug | Status: new
Component: Database | Version: 5.0
layer (models, ORM) |
Severity: Release | Keywords: field, database,
blocker | generated, output_field
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Broken SQL code generated when KeyTextTransform is used in GeneratedField
expression

**Steps**

Steps to reproduce the error.

**Model**

{{{#!python

from django.db import models
from django.db.models.fields.json import KT

class Response(models.Model):
data = models.JSONField(default=dict)
status = models.GeneratedField(
db_persist=True,
expression=KT("data__status"),
output_field=models.PositiveSmallIntegerField(),
)
}}}

**Migration**

{{{#!bash
$ python3 -m manage makemigrations
Migrations for 'https':
https/migrations/0001_initial.py
- Create model Response
$ python -m manage sqlmigrate https 0001
}}}

{{{#!sql
BEGIN;
--
-- Create model Response
--
CREATE TABLE "https_response" (
"id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
"data" jsonb NOT NULL,
"status" smallint GENERATED ALWAYS AS (None("data")) STORED
);
COMMIT;
}}}

**Traceback**

{{{#!bash
$ python -m manage migrate https 0001
}}}

{{{#!python
Operations to perform:
Target specific migration: 0001_initial, from https
Running migrations:
Applying https.0001_initial...Traceback (most recent call last):
File "/home/paulox/Projects/generatedfield/.venv/lib/python3.11/site-
packages/django/db/backends/utils.py", line 99, in _execute
return self.cursor.execute(sql)
^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/paulox/Projects/generatedfield/.venv/lib/python3.11/site-
packages/psycopg/cursor.py", line 737, in execute
raise ex.with_traceback(None)
psycopg.errors.SyntaxError: syntax error at or near "("
LINE 1: ... NULL, "status" smallint GENERATED ALWAYS AS (None("data")) ...
^
}}}

**Queryset**

Using KeyTextTransform in a query instead generates a correct SQL code.

{{{#!pycon
>>> from django.db.models.fields.json import KT
>>> from https.models import Response
>>> str(Response.objects.values_list(KT("data__status")).query)
'SELECT ("https_response"."data" ->> status) AS "keytexttransform1" FROM
"geometricfigures_response"'
}}}

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

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------

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: Accepted
generated, output_field |
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Unreviewed => Accepted

Comment:

The issue lies in `GeneratedField.generated_sql`, it should not call
`as_sql` directly as it doesn't account for `{vendor}_sql` overrides.

It should let the compiler compile the expression instead, something like

{{{#!diff
diff --git a/django/db/models/fields/generated.py
b/django/db/models/fields/generated.py
index deb5875638..225d3e9d12 100644
--- a/django/db/models/fields/generated.py
+++ b/django/db/models/fields/generated.py
@@ -61,12 +61,10 @@ def contribute_to_class(self, *args, **kwargs):
self.register_lookup(lookup, lookup_name=lookup_name)

def generated_sql(self, connection):
- return self._resolved_expression.as_sql(
- compiler=connection.ops.compiler("SQLCompiler")(
- self._query, connection=connection, using=None
- ),
- connection=connection,
+ compiler = connection.ops.compiler("SQLCompiler")(
+ self._query, connection=connection, using=None
)
+ return compiler.compile(self._resolved_expression)

def check(self, **kwargs):
databases = kwargs.get("databases") or [
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34861#comment:1>

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------

Reporter: Paolo Melchiorre | Owner: Paolo
| Melchiorre
Type: Bug | Status: assigned

Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: field, database, | Triage Stage: Accepted
generated, output_field |

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Paolo Melchiorre):

* owner: nobody => Paolo Melchiorre
* status: new => assigned
* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/34861#comment:2>

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------
Reporter: Paolo Melchiorre | Owner: Paolo
| Melchiorre
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: field, database, | Triage Stage: Accepted
generated, output_field |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Paolo Melchiorre):

Replying to [comment:1 Simon Charette]:

> The issue lies in `GeneratedField.generated_sql`, it should not call
`as_sql` directly as it doesn't account for `{vendor}_sql` overrides.
>
> It should

[https://github.com/django/django/blob/78b5c9075348aa12da2e024f6ece29d1d652dfdd/django/db/models/sql/compiler.py#L541-L547
let the compiler compile the expression instead], something like

Thanks, Simon, I used the code you suggested and confirm that it solves
the problem.

I tested the patch locally on my PC with both SQLite and PostgreSQL.

I submitted a commit with the fix where I added you as a co-author and
tried to add an explicit test on the generated SQL code.

Here you can find my PR https://github.com/django/django/pull/17301

--
Ticket URL: <https://code.djangoproject.com/ticket/34861#comment:3>

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------
Reporter: Paolo Melchiorre | Owner: Paolo
| Melchiorre
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: field, database, | Triage Stage: Accepted
generated, output_field |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Thanks for the patch and the tests Paolo!

--
Ticket URL: <https://code.djangoproject.com/ticket/34861#comment:4>

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------
Reporter: Paolo Melchiorre | Owner: Paolo
| Melchiorre
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: field, database, | Triage Stage: Accepted
generated, output_field |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Paolo Melchiorre):

Replying to [comment:4 Simon Charette]:

> Thanks for the patch and the tests Paolo!

Thank you for your invaluable suggestions.

--
Ticket URL: <https://code.djangoproject.com/ticket/34861#comment:5>

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------
Reporter: Paolo Melchiorre | Owner: Paolo
| Melchiorre
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: field, database, | Triage Stage: Ready for
generated, output_field | checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/34861#comment:6>

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------
Reporter: Paolo Melchiorre | Owner: Paolo
| Melchiorre

Type: Bug | Status: closed

Component: Database layer | Version: 5.0
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: field, database, | Triage Stage: Ready for
generated, output_field | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"574ee4023e15cfb195833edfbaed353f8021c62f" 574ee402]:
{{{
#!CommitTicketReference repository=""
revision="574ee4023e15cfb195833edfbaed353f8021c62f"
Fixed #34861 -- Fixed crash when adding GeneratedField with some
expressions.

Co-authored-by: Simon Charette <chare...@gmail.com>
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34861#comment:7>

[Django]

#34861: KeyTextTransform incompatible with GeneratedField
-------------------------------------+-------------------------------------
Reporter: Paolo Melchiorre | Owner: Paolo
| Melchiorre
Type: Bug | Status: closed
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: field, database, | Triage Stage: Ready for
generated, output_field | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"81663cc4ca8f7ac0eac33cbb6986462330e75bb1" 81663cc4]:
{{{
#!CommitTicketReference repository=""
revision="81663cc4ca8f7ac0eac33cbb6986462330e75bb1"
[5.0.x] Fixed #34861 -- Fixed crash when adding GeneratedField with some
expressions.

Co-authored-by: Simon Charette <chare...@gmail.com>

Backport of 574ee4023e15cfb195833edfbaed353f8021c62f from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34861#comment:8>