[Django] #24509: Allow Expressions when saving new models
Django
----------------------------------------------+--------------------
Reporter: AlexHill | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Opening a ticket after discussion on IRC with Josh.
You can use `Expression` when updating models, but it would also be useful
to be able to use them when creating models, particularly in order to
apply database functions to newly-inserted values.
My specific use case for this is a PostgreSQL `TSVectorField`, where
instead of just inserting a value, I want to insert
`strip(to_tsvector(<ts_config>, unaccent(<value>)))`. Ideally, I would be
able to define these all as `Func`s and insert an `Expression`.
I propose to fix this by making `SQLInsertCompiler` check for
`resolve_expressions` and `as_sql`, as `SQLUpdateCompiler` does.
Alex
--
Ticket URL: <https://code.djangoproject.com/ticket/24509>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: AlexHill | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
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 akaariai):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Sounds like a good idea to me. This will also be one step towards database
based default values which could be a nice feature.
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:1>
Django
Reporter: AlexHill | Owner: r4vi
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by r4vi):
* status: new => assigned
* owner: nobody => r4vi
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:2>
Django
Reporter: AlexHill | Owner: r4vi
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by r4vi):
I've started to look at this but if anyone knows this part of the codebase
well then I think they'd be able to add this quite quickly so feel free to
re-assign this if you want to work on it
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:3>
Django
Reporter: AlexHill | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by r4vi):
* status: assigned => new
* owner: r4vi =>
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:4>
Django
Reporter: AlexHill | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by adamchainz):
* cc: me@… (added)
Comment:
I also have a use-case for this - I'm trying to implement MariaDB Dynamic
Columns which require calling a function to create (
https://mariadb.com/kb/en/mariadb/dynamic-columns/#column_create ).
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:5>
Django
Reporter: AlexHill | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
Just for future reference for whoever works on this: I spent a few minutes
looking at this tonight and I think that implementing this _might_ allow
us to get rid of the whole `Field.get_placeholder()` hook, which is
effectively just a hacky way to allow fields to wrap their values in an
arbitrary SQL expression at save time. (Currently only GIS fields use this
hook in core.)
In the meantime, if you're wishing Django 1.8 had this feature,
`get_placeholder()` appears to be a usable workaround (for any given
specific field type; doesn't offer generic support).
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:6>
Django
Reporter: AlexHill | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by AlexHill):
* has_patch: 0 => 1
Old description:
> Opening a ticket after discussion on IRC with Josh.
>
> You can use `Expression` when updating models, but it would also be
> useful to be able to use them when creating models, particularly in order
> to apply database functions to newly-inserted values.
>
> My specific use case for this is a PostgreSQL `TSVectorField`, where
> instead of just inserting a value, I want to insert
> `strip(to_tsvector(<ts_config>, unaccent(<value>)))`. Ideally, I would be
> able to define these all as `Func`s and insert an `Expression`.
>
> I propose to fix this by making `SQLInsertCompiler` check for
> `resolve_expressions` and `as_sql`, as `SQLUpdateCompiler` does.
>
> Alex
New description:
Opening a ticket after discussion on IRC with Josh.
You can use `Expression` when updating models, but it would also be useful
to be able to use them when creating models, particularly in order to
apply database functions to newly-inserted values.
My specific use case for this is a PostgreSQL `TSVectorField`, where
instead of just inserting a value, I want to insert
`strip(to_tsvector(<ts_config>, unaccent(<value>)))`. Ideally, I would be
able to define these all as `Func`s and insert an `Expression`.
I propose to fix this by making `SQLInsertCompiler` check for
`resolve_expressions` and `as_sql`, as `SQLUpdateCompiler` does.
Alex
Update: patch at https://github.com/django/django/pull/5099
--
Comment:
Here's my patch implementing this:
https://github.com/django/django/pull/5099
It also adds support for expressions (and placeholders) in bulk inserts.
Notes:
* As Carl suggested, this should allow us to get rid of
`get_placeholder()` entirely.
* The handling of `AutoField`s is a bit ugly at the moment. If an
`AutoField`'s value is `None`, it's removed from the list of fields in
`Model._save_table()` and then conditionally re-added in
`SQLInsertCompiler.as_sql()`. I'm thinking about nicer ways to handle
this.
* There is a heap of same-but-different code between `SQLInsertCompiler`
and `SQLUpdateCompiler`, especially now that expressions are supported.
I'd like to clean this up in a subsequent patch.
* I've changed the workaround for #10888. I'm pretty sure it won't change
anything, but I can't test in Oracle.
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:7>
Django
Reporter: AlexHill | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:8>
Django
Reporter: AlexHill | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jarshwah):
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
Comment:
https://github.com/django/django/pull/5324 is a docs update + rebase.
Ready for better review. I haven't reviewed close enough to say whether or
not this should make 1.9 though. If other ORM people are around, please
feel free to comment. I'm hoping to get an hour later to properly go over
this.
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:9>
Django
Reporter: AlexHill | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:10>
Django
Reporter: AlexHill | Owner: Josh
| Smeaton <josh.smeaton@…>
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Josh Smeaton <josh.smeaton@…>):
* owner: => Josh Smeaton <josh.smeaton@…>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"134ca4d438bd7cbe8f0f287a00d545f96fa04a01" 134ca4d]:
{{{
#!CommitTicketReference repository=""
revision="134ca4d438bd7cbe8f0f287a00d545f96fa04a01"
Fixed #24509 -- Added Expression support to SQLInsertCompiler
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24509#comment:11>