Opened 5 years ago

Last modified 8 months ago

#31383 new Cleanup/optimization

Make createcachetable use SchemaEditor for SQL generation

Reported by: Tim Graham Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Normal Keywords:
Cc: Ülgen Sarıkavak Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The createcachetable management command generates its SQL manually. The generated 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:

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.

Attachments (1)

31383.diff (3.4 KB ) - added by Tim Graham 5 years ago.
draft patch

Download all attachments as: .zip

Change History (5)

by Tim Graham, 5 years ago

Attachment: 31383.diff added

draft patch

comment:1 by Tim Graham, 5 years ago

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.

comment:2 by Simon Charette, 5 years ago

Triage Stage: UnreviewedAccepted

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 handle it if it's allowed by the underlying backend.

comment:3 by Tim Graham, 5 years ago

Has patch: set
Patch needs improvement: set

PR (tests not passing yet)

comment:4 by Ülgen Sarıkavak, 8 months ago

Cc: Ülgen Sarıkavak added
Note: See TracTickets for help on using tickets.
Back to Top