Opened 19 years ago

Last modified 11 months ago

#470 closed New feature

Add Field.db_default for defining database defaults — at Version 42

Reported by: jws Owned by: Ian Foote
Component: Database layer (models, ORM) Version: dev
Severity: normal Keywords: sql schema
Cc: Ryan Hiebert, Ian Foote, Ryan Moore, Václav Řehák, Hannes Ljungberg, Marcin Nowak, Adrian Turjak, Doug Harris, Leigh Brenecki, Johannes Maron, Charlie Denton, Matt Goldman, raydeal, Todor Velichkov, Alex Scott, David Sanders, Lily Foote, bcail, Adrian Torres, Michael Rosner 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 Anders Hovmöller)

https://github.com/django/django/pull/13709

Apply this diff to django/core/mamagement.py

Should work on any database.

75a76,77

if f.default <> meta.fields.NOT_PROVIDED:

field_output.append("DEFAULT '%s'" % (f.default,))

Change History (46)

by jws, 19 years ago

Attachment: management.diff added

comment:1 by Adrian Holovaty, 19 years ago

Summary: diff to cause 'default' values to be expressed in sql schema[patch] "default" values should be expressed in SQL schema

comment:2 by Adrian Holovaty, 18 years ago

Resolution: wontfix
Status: newclosed

I'm marking this as a wontfix because we don't have a way of converting Python objects to SQL-friendly syntax, for insertion into the "DEFAULT" clause in a CREATE TABLE statement. For example, the default value "John's test", which has a quote in it, may have to be represented differently in SQL, depending on the backend. Unfortunately, not all of the database modules (psycopg, MySQLdb, etc.) expose functionality that quotes the values.

comment:3 by jws, 18 years ago

Resolution: wontfix
Status: closedreopened

GOAL:
Django should express the structure and relationships of the models in the database schema as fully as possible within the capabilities of a specific backend. This is important when working with other tools that do not use the Django ORM.

PROBLEM:
When django-admin creates sql statements in response to the 'sql','sqlclear' and related sub-commands, string concatenation is used rather than the parameterized queries that are used in the normal object manipulation code. If default values for fields contain delimiting characters(',",;,etc), the resulting sql will contain errors. The string describing the default value must be processed by a backend-specific character-escaping method that will alter any problem characters before it can be inserted into the sql statement.

SOLUTION:
When describing a field with a default value, if the backend has a method called escapechars(), a sql 'DEFAULT' clause is assembled and inserted. If no such method exists, no additional clause is inserted, though the Django objects still produce default values.

IMPLEMENTATION:
Postgresql: Updated and tested
Sqlite: Updated, not tested
Mysql: Not updated
MS-MSQL: Not updated

Extra notes:

pymysqldb exposes a quoting function that could simply be wrapped.

I'll update this patch again when I get the ability to test the other backends. Help is appreciated. However, patch only adds new functionality. Unmodified backends should continue to work as before.

by jws, 18 years ago

Attachment: 470-2.patch added

Second rev, with addition of escaping function

comment:4 by Jacob, 18 years ago

Resolution: wontfix
Status: reopenedclosed

Re-marking as wontfix.

by Jws, 18 years ago

Attachment: 470-2813.patch added

patch updated for pending release of 0.95

comment:5 by jws, 18 years ago

Resolution: wontfix
Status: closedreopened

I have updated my patch to the current svn with magic-removal mainlined in. The patch now supports all backends, as well. I would like to get this in in time for the 0.95 release.

by jws, 18 years ago

Attachment: 470-2900.diff added

update for 2900

comment:6 by anonymous, 18 years ago

milestone: Version 0.92
Version: 0.91

comment:7 by newpers, 18 years ago

it would be nice to see this merged into the trunk. as i stated in #django, an ORM can't do everything so there is a need for custom sql. it frustrates me when i do an INSERT, but it doesn't know what to default to. try doing:

I actually needed this today: INSERT INTO tbl DEFAULT VALUES;

comment:8 by Gary Wilson <gary.wilson@…>, 18 years ago

milestone: Version 0.92Version 1.0

0.92 is long gone.

comment:9 by Jacob, 17 years ago

Resolution: wontfix
Status: reopenedclosed

Again, this is wontfix for the reasons Adrian articulated.

comment:10 by (none), 17 years ago

milestone: Version 1.0

Milestone Version 1.0 deleted

comment:11 by Cedric Shock <cedric@…>, 17 years ago

Patch needs improvement: set
Resolution: wontfix
Status: closedreopened

I am reopening this as the reasons Adrian articulated were addressed in later patches without further rebuttal. Default values would be very useful in introspective upgrading of databases, although other means of providing values to new fields per existing row through a more complete upgrade architecture would also be desirable.

I have a few issues with how this patch works. The first is that the user is to provide the default value in the format of a string to the database, not in the format of a value of the field. This can be solved easily with the Field get_db_prep_save:

newest patch, management,py, new line 173:

escaped_string =  django.db.backend.escapechars(f.get_db_prep_save(f.default))

Furthermore the code makes the assumption that values shall be quoted with single quotes. It would be more appropriate for the escapechars interface to return an entire quoted string usable as a value (move the single quotes into each escapechars function, making new line 174 in the same file like:)

field_output.append(style.SQL_KEYWORD("DEFAULT %s" % (escaped_string,)))

Furthermore, the implementation of the escapechars (or otherwise named) function should be done as much as is possible and appropriate using routines provided by the various backends (psycodb, etc.).

The DB-API way of dealing with escaped values is via the cursor execute. That would change these two lines to something like:

field_output.append(style.SQL_KEYWORD("DEFAULT %%s")
field_output_params.append(f.get_db_prep_save(f.default))

However this last way would require complete reworking of the management.py everything to support passing these params around with the sql strings, and emulation of DB-API execute substitution, for each backend, to output SQL statements. It would be nice if the DB-API interface also provided something like execute_sql(...) and executemany_sql(...) which returned strings such that execute(execute_sql(...)) and execute(...) were functionally equivalent.

I'd also be tempted to rename escapechars to escapedefault due to its very specific intent, or to escapeparam if it is intended to work on what DB-API calls parameters in general.

Furthermore, default values for fields should be reflected in the admin interface.

comment:12 by Simon G. <dev@…>, 17 years ago

Triage Stage: UnreviewedDesign decision needed
Version: 0.91

& once again around the Design-Decision-needed loop...

comment:13 by James Bennett, 17 years ago

Still needs to address the issue of callable defaults -- since those are calculated on the fly at the time of insertion, it's not going to be possible to express them in SQL.

comment:14 by Jacob, 17 years ago

Resolution: wontfix
Status: reopenedclosed

Once again, I'm marking wontfix. Let's bring this up on django-dev if more debate is needed.

comment:15 by aron45, 10 years ago

Easy pickings: unset
UI/UX: unset

Hi, bumping up here old ticket.
I think this is a very important feature, as I'm writing the sql my self for this reason.

any solutions?

comment:16 by Marcin Nowak, 6 years ago

Please reopen it and implement using INSERT ... RETURNING (for postgres). This may be optional behaviour dependent on db engine used.
Defaults computed at the database level are very important. This may be optional feature, but it should be available. The database is most important and long-lived part of any bigger system.

Python-like callbacks passed as defaults shouldn't be supported, of course. They may behave same as before and they should not be listed in RETURNING clause.

comment:17 by Shai Berger, 6 years ago

There was a discussion on this topic, with some partial work to support the feature, about two years ago: https://groups.google.com/d/topic/django-developers/3mcro17Gb40/discussion

comment:18 by Tim Graham, 6 years ago

Has patch: unset
Patch needs improvement: unset
Summary: [patch] "default" values should be expressed in SQL schemaAdd Field.db_default for defining database defaults
Triage Stage: Design decision neededAccepted
Type: enhancementNew feature

Reopening as per the mailing list discussion in the previous comment.

comment:19 by Tim Graham, 6 years ago

Resolution: wontfix
Status: closednew

comment:20 by Paul Tiplady, 6 years ago

Further justification for this feature -- it makes zero-downtime DB migrations easier: #29266.

Last edited 6 years ago by Tim Graham (previous) (diff)

comment:21 by Ryan Hiebert, 6 years ago

Cc: Ryan Hiebert added

comment:22 by Simon Charette, 5 years ago

Cc: Simon Charette added

comment:23 by Ian Foote, 5 years ago

Cc: Ian Foote added

comment:24 by Ryan Moore, 5 years ago

Cc: Ryan Moore added

comment:25 by Václav Řehák, 4 years ago

Cc: Václav Řehák added

comment:26 by Hannes Ljungberg, 4 years ago

Cc: Hannes Ljungberg added

comment:27 by Marcin Nowak, 4 years ago

Cc: Marcin Nowak added

comment:28 by Marcin Nowak, 4 years ago

Hi.

Django 3.x generates sql with drop default:

        ALTER TABLE "x" ADD COLUMN "y" integer DEFAULT 0 NOT NULL CHECK ("y" >= 0);                                
        ALTER TABLE "x" ALTER COLUMN "y" DROP DEFAULT;  

The whole thing is about not adding DROP DEFAULT. How about some kind of option for database backend?

Last edited 4 years ago by Marcin Nowak (previous) (diff)

comment:29 by Vaibhav Awachat, 4 years ago

Currently using this https://github.com/3YOURMIND/django-add-default-value to run migrations without downtime using MySQL in strict mode.

comment:30 by Marcin Nowak, 4 years ago

Hi. 15 years have passed since this important bug report. Every time we're uploading new version of the application, which includes new not nullable fields in db tables, our service is failing. Db changes are applied first, then app services are restarting on all servers (it takes few minutes). During this time old version loaded into memory is failing due to missing defaults:

IntegrityError
null value in column "X" violates not-null constraint DETAIL: Failing row contains (...)

For us, default values must be set at the database layer. Please add support of db_default attribute.

comment:31 by Simon Charette, 4 years ago

Cc: Simon Charette removed

comment:32 by Adrian Turjak, 3 years ago

Cc: Adrian Turjak added

comment:33 by Doug Harris, 3 years ago

Cc: Doug Harris added

comment:34 by Ian Foote, 3 years ago

Owner: changed from nobody to Ian Foote
Status: newassigned

comment:35 by Ian Foote, 3 years ago

Has patch: set
Needs documentation: set

comment:36 by Leigh Brenecki, 3 years ago

Cc: Leigh Brenecki added

comment:37 by Ian Foote, 3 years ago

Needs documentation: unset

comment:38 by Johannes Maron, 3 years ago

Cc: Johannes Maron added

comment:39 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

comment:40 by Charlie Denton, 3 years ago

Cc: Charlie Denton added

comment:41 by Anders Hovmöller, 3 years ago

I am getting bitten by this quite a lot.

I would suggest that long term it would be a good idea to have one field for the default value set in the database schema and one that is dynamic and set on the Django level. Maybe default and default_db to keep backwards compatibility, or default_dynamic and default to improve the default.

comment:42 by Anders Hovmöller, 3 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top