keredson / peewee

a small, expressive orm -- supports postgresql, mysql and sqlite
http://docs.peewee-orm.com/
MIT License
13 stars 4 forks source link

peewee doesn't support real database defaults #8

Open keredson opened 8 years ago

keredson commented 8 years ago

When you create a table with a default column in peewee:

class Person(pw.Model):
  name = pw.CharField(default='Frank')
  class Meta:
    database = db

db.create_tables([Person])

Peewee don't actually set the column default:

CREATE TABLE person (
  id SERIAL NOT NULL PRIMARY KEY, 
  "name" VARCHAR(255) NOT NULL
)

This is mentioned briefly in a note deep in the documentation:

Both default and choices could be implemented at the database level as DEFAULT and CHECK CONSTRAINT respectively, but any application change would require a schema change. Because of this, default is implemented purely in python and choices are not validated but exist for metadata purposes only.

But not very prominently, nor really explained. (The behavior is explained, but why that behavior was chosen is not, or at least I don't understand "but any application change would require a schema change" enough to evaluate it. If someone has insight I'd love to hear.)

I don't think this is the correct behavior for an ORM. A "default" in the context of a database field is well established. To quietly override that semantic is dangerous for an ORM and can lead to surprising behavior. (see https://github.com/coleifer/peewee/issues/940, https://github.com/coleifer/peewee/issues/111, https://github.com/coleifer/wtf-peewee/issues/26, https://github.com/coleifer/peewee/issues/1048, https://github.com/coleifer/peewee/issues/34, https://github.com/coleifer/peewee/issues/909, https://github.com/coleifer/peewee/issues/935, https://github.com/coleifer/peewee/issues/543 and many others on stackoverflow)

IMO, setting default on a column should define the database schema default, and if you want a python-only version of the same, a second option python_default should have been created (to clearly differentiate it's behavior).

But how do we get there from here without breaking compatibility all to heck?

I propose two new kwargs for Field:

  1. python_default - works as the current default does.
  2. db_default - sets the default in the database.

Either (or both) could be set explicitly by the user to force one behavior or the other.

The current default could be interpreted by peewee via a heuristic. If the value passed was one of:

The value passed to default would be used for db_default (if not already defined). Otherwise it would be passed to python_default (if not already defined).

This would Do The Right Thing(tm) most of the time, be pretty backwards compatible, and let the user override the heuristic if they truly know what they want.

I don't know the actual side effects of doing this in peewee's internals, obviously I'll cross that bridge once I start trying to implement. :)

@obensonne @bcattle @staier @Dongin @jonklein @mmongeon-aa since these were your issues i'm referencing.

@smtheard @neopunisher @trichu1988 @jgarcia since we were all talking about it.

Thoughts / suggestions?