Issue: Pwiz does not generate column defaults
Author: geonyoro
State: closed
Created: 2018-03-28 21:04:16

There seems to be no inclusion of column default values. 

Given that the information schema for the three systems includes it, is it okay if I submit my workaround for this as a pull request?
My pull initially focuses on MySQL, but with 'possibility' of including psql and sqlite.

I'm curious what this patch would entail...feel free to submit but I can't make any guarantees about merging.

I am not sure whether any defaults extracted from the schema itself should be translated into Python defaults. Enforcement at the schema level is different than at the application level.

Looking at postgres output:

```# \d eventlog
                                     Table "public.eventlog"
  Column   |            Type             |                       Modifiers                       
-----------+-----------------------------+-------------------------------------------------------
 id        | integer                     | not null default nextval('eventlog_id_seq'::regclass)
 timestamp | timestamp without time zone | not null default CURRENT_TIMESTAMP
 data      | text                        | not null
```

Output of:

```sql
select column_name, ordinal_position, column_default, is_nullable, data_type
from information_schema.columns 
where table_name='eventlog';

 column_name | ordinal_position |            column_default            | is_nullable |          data_type          
-------------+------------------+--------------------------------------+-------------+-----------------------------
 id          |                1 | nextval('eventlog_id_seq'::regclass) | NO          | integer
 timestamp   |                2 | CURRENT_TIMESTAMP                    | NO          | timestamp without time zone
 data        |                3 |                                      | NO          | text
(3 rows)
```

Looking at this output, we'd want to ignore the `nextval(...)` for the "id" column, since this is just the auto-generated sequence. As for the `CURRENT_TIMESTAMP`, this raises the question...would this be translated to `default=datetime.datetime.now`, or more correctly listed as a column constraint, e.g.:

```python
timestamp = DateTImeField(constraints=[SQL('DEFAULT CURRENT_TIMESTAMP')])
```

Well, I can see the problem as highlighted for postgres. 

For the rest, I propose just doing the bare minimum to get it working for the simple data types, float, integers, datetime(as proposed above).

My thoughts are that for anyone who is using peewee for extended testing, the db schema generated by the using create_tables should be the same as an original schema taken and then having the models file created by pwiz. ie.

db1 -> pwiz -> models.py
models.py > create_tables < db2
db2 == db1 
as close as possible.

If both db1 and db2 are generated by the peewee file, then sure, they are likely equal, but if the above process is used, they might be different.
I'm more partial to Sqlite and MySQL since those are what I use.

> For the rest, I propose just doing the bare minimum to get it working for the simple data types, float, integers, datetime(as proposed above).

Doing something halfway can often lead to a worse result than not doing it at all.

I'm also still not sure whether the default column values should be expressed as constraints (which is more "correct") or as application-level Python defaults. Implementing them as server constraints would preserve the symmetry between introspection -> models.py -> create_tables(), but might also lead to undesired application-level behavior since the server-side defaults may not be communicated back to the application layer.

For simple values like string/numeric constants I think it might be better to implement them as application-level Python defaults. For functions/expressions like `CURRENT_TIMESTAMP` (or `now()` in postgres), it would be tempting to try and convert these to a Python equivalent like `datetime.datetime.now`, wherever possible... But doing this at the application level would break the symmetry between the reflected db and the schema peewee would create with the introspected models.

I've got a sample patch that captures the server-side constraint and preserves it as a column constraint: 93ad80375f23181cac71836c81097b4ba8f09957

Another option occurred to me, which is to let the user decide by specifying command-line options...e.g. `--server-defaults` and `--python-defaults` or something.

Well, dang it bro, that was fast. I was hoping to use this as my "chance" to say I've worked on something open source by contributing to the code, but hey, I guess some other time.

Anyway, server defaults I think "should" already be permanent. The python defaults at such a point don't mean much then.

Use case: if we are running tests on an empty DB, for whatever reason, the testing schema should be close to reflected as possible. That's the challenge I face.

In what situation would server side defaults not be communicated back to the application layer after introspection -> models -> create table? 

Well, let's say you have a timestamp column that defaults to the current timestamp. If you create a model instance without specifying the timestamp, letting the db determine the value, the timestamp for the new row is not communicated back to the application. You'd need to re-fetch the value from the newly-inserted row to get the timestamp. This isn't necessarily true with Postgres, which supports the `RETURNING` clause, but something to be aware of.

As far as `pwiz`, it is more intended to save you time when starting to use peewee with an existing database. So the assumption is that you'll fill in the `default` settings, etc, and use the generated code as a starting point rather than the final thing.

For a test database, you shouldn't need to use pwiz since presumably your application already has all the models?

a66c54c now includes change to capture server-side defaults.

Cool!

By the way, this issue was generated by the recommended method in the testing section of [the documentation](http://docs.peewee-orm.com/en/latest/peewee/database.html), where it is recommended 

> As an aside, and speaking from experience, I recommend testing your application using the same database backend you use in production, so as to avoid any potential compatibility issues.

And in the example:
`
        test_db.connect()
        test_db.create_tables(MODELS)`

That's right... I don't see where `pwiz` would enter into the equation when testing using your application models, however?