Opened 3 years ago

Closed 2 years ago

#24509 closed New feature (fixed)

Allow Expressions when saving new models

Reported by: Alex Hill Owned by: Josh Smeaton <josh.smeaton@…>
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: me@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Alex Hill)

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 Funcs 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

Change History (11)

comment:1 Changed 3 years ago by Anssi Kääriäinen

Triage Stage: UnreviewedAccepted

Sounds like a good idea to me. This will also be one step towards database based default values which could be a nice feature.

comment:2 Changed 3 years ago by Ravi Kotecha

Owner: changed from nobody to Ravi Kotecha
Status: newassigned

comment:3 Changed 3 years ago by Ravi Kotecha

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

comment:4 Changed 3 years ago by Ravi Kotecha

Owner: Ravi Kotecha deleted
Status: assignednew

comment:5 Changed 3 years ago by Adam (Chainz) Johnson

Cc: me@… added

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 ).

comment:6 Changed 3 years ago by Carl Meyer

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).

comment:7 Changed 2 years ago by Alex Hill

Description: modified (diff)
Has patch: set

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 AutoFields 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.

comment:8 Changed 2 years ago by Tim Graham

Needs documentation: set
Patch needs improvement: set

comment:9 Changed 2 years ago by Josh Smeaton

Needs documentation: unset
Patch needs improvement: unset

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.

comment:10 Changed 2 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:11 Changed 2 years ago by Josh Smeaton <josh.smeaton@…>

Owner: set to Josh Smeaton <josh.smeaton@…>
Resolution: fixed
Status: newclosed

In 134ca4d:

Fixed #24509 -- Added Expression support to SQLInsertCompiler

Note: See TracTickets for help on using tickets.
Back to Top