default values like datetime.datetime.now break sql output

[anonymous]

just checking against basestring in column_sql isn't robust enough

[iacobs@…]

Not adding a default value if field.default is callable should suffice IMHO, that's what the Django ORM seems to do.

This worked for me:

Index: db/generic.py
===================================================================
--- db/generic.py	(revision 74)
+++ db/generic.py	(working copy)
@@ -141,7 +141,7 @@
         sqlparams = ()
         # if the field is "NOT NULL" and a default value is provided, create the column with it
         # this allows the addition of a NOT NULL field to a table with existing rows
-        if not field.null and field.has_default():
+        if not field.null and field.has_default() and not callable(field.default):
             default = field.get_default()
             if isinstance(default, basestring):
                 default = "'%s'" % default

[amccurdy]

There's one piece missing here. If the column is marked as NOT NULL, then the db column must be provided a default value. Otherwise, the database, at least in PG's case, will complain about the column containing NULL values.

I had thought previously that running the alter in a transaction, followed by an update query to fix the fields prior to the commit would fix this behavior, but that doesn't seem to be working.

[amccurdy]

I think we'd have to do something like this:

andy=# start transaction;
START TRANSACTION
andy=# alter table foo add foo_date timestamp with time zone NULL;
ALTER TABLE
andy=# update foo set foo_date = RESULT_OF_PYTHON_CALLABLE;  
UPDATE 11
andy=# alter table foo alter column foo_date set not null;
ALTER TABLE
andy=# commit;
COMMIT
andy=# 

[andrew]

Yes, Andy, I'd agree with that; it's the simplest translation of the whole process.

We could either do this automatically, or just raise an error when you try to add a NOT NULL column with a callable as a default (or, in fact, no default, as currently you get nasty SQL errors). The former seems nicer from a migration perspective, but I'm not sure where my "too much magic" stance lies here.

[amccurdy]

I don't think this is "too much magic". In fact, it's exactly what happens within the Postgres black box when we specify a literal default value. The column gets added, and all the existing rows get updated.

[andrew]

Yes, good point; we'll go ahead and implement it as you suggested. I'll have a stab at it tomorrow.

[andrew]

Fixed in [125]. The whole drop-default-if-we-don't-need-it part is already done as part of add_column, but not create_table; that's potentially another ticket.