[Django] #31383: Make createcachetable use SchemaEditor for SQL generation

[Django]

#31383: Make createcachetable use SchemaEditor for SQL generation
-------------------------------------+-------------------------------------
Reporter: Tim | Owner: nobody
Graham |
Type: | Status: new
Cleanup/optimization |
Component: Core | Version: master
(Management commands) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The createcachetable management command
[https://github.com/django/django/blob/c1c361677d9400c8e2cdaddda0c16086bb358492/django/core/management/commands/createcachetable.py#L65-L87
generates its SQL manually]. The generated SQL:
{{{ #!sql
CREATE TABLE test_cache_table (
cache_key STRING(255) NOT NULL PRIMARY KEY,
value STRING(MAX) NOT NULL,
expires TIMESTAMP NOT NULL
)
}}}
is incompatible with Google's Cloud Spanner databases, which requires a
different PRIMARY KEY syntax:
{{{ #!sql
CREATE TABLE test_cache_table (
cache_key STRING(255) NOT NULL,
value STRING(MAX) NOT NULL,
expires TIMESTAMP NOT NULL
) PRIMARY KEY (cache_key)
}}}
This issue wouldn't happen if the management command used
`SchemaEditor.create_model()` for SQL generation.

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

[Django]

#31383: Make createcachetable use SchemaEditor for SQL generation
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

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

Changes (by Tim Graham):

* Attachment "31383.diff" added.

draft patch

[Django]

#31383: Make createcachetable use SchemaEditor for SQL generation
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: nobody
Type: | Status: new
Cleanup/optimization |

Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed

Has patch: 0 | Needs documentation: 0

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

Comment (by Tim Graham):

The draft patch seems to work except for "Executing DDL statements while
in a transaction on databases that can't perform a rollback is
prohibited." (from 813805833aca60544f6b3d686c015039f6af5d1d) when running
in non-dry-run mode.

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

[Django]

#31383: Make createcachetable use SchemaEditor for SQL generation
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: nobody
Type: | Status: new
Cleanup/optimization |

Component: Core (Management | Version: master
commands) |
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 Simon Charette):

* stage: Unreviewed => Accepted

Comment:

It's a shame that the schema editor cannot operate on model states and
requires the creation of a temporary model here.

You'll want to use a throw away `apps` instead of a `dummy` app name,
here's how the schema recorder does it

https://github.com/django/django/blob/c1c361677d9400c8e2cdaddda0c16086bb358492/django/db/migrations/recorder.py#L30-L43

In order to address `Executing DDL statements while in a transaction on
databases that can't perform a rollback is prohibited.` you should remove
the outer `atomic` block and let `SchemaEditor`
[https://github.com/django/django/blob/c1c361677d9400c8e2cdaddda0c16086bb358492/django/db/backends/base/schema.py#L101
handle it] if it's allowed by the underlying backend.

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

[Django]

#31383: Make createcachetable use SchemaEditor for SQL generation
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: nobody
Type: | Status: new
Cleanup/optimization |

Component: Core (Management | Version: master
commands) |
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
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* needs_better_patch: 0 => 1
* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/12590 PR] (tests not passing yet)

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

[Django]

#31383: Make createcachetable use SchemaEditor for SQL generation
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Management | Version: dev

commands) |
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
-------------------------------------+-------------------------------------

Changes (by Ülgen Sarıkavak):

* cc: Ülgen Sarıkavak (added)

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