Phinx: Phinx default for null columns contravenes DBMS default

[Bilge]

By default, Phinx creates columns as NOT NULL, but the default in Postgres is NULL. Consider the following query.

ALTER TABLE public.foo
    ADD COLUMN bar timestamp with time zone;

This query creates the column bar but does not specify any NULL options. By default it creates the column permitting nulls. In contrast, the following Phinx query will do the opposite:

$this->table('foo')->addColumn('bar', 'timestamp')->update();

Not only does this betray expectations, it also causes errors if there's existing rows.

PDOException: SQLSTATE[23502]: Not null violation: 7 ERROR: column "bar" contains null values

I believe these defaults should be aligned to avoid unpleasant surprises.

[MasterOdin]

It should be noted this affects all adapters equally, and not just postgres.

What would be the preference for identity columns? The default id one should be NOT NULL, but how about user specified columns where they have:

    ->addColumn('id', 'integer', ['identity' => true])

Should these also be null by default, or not null? SQLAlchemy at least defaults to not null for identity columns, and null for all other cases, not sure about others.

[Bilge]

Primary keys cannot be null in Postgres, and in the newer version of MySQL, this is the same. This behaviour is specified by the SQL-89 and SQL-92 standards. Whilst this is not true for all DBMS, e.g. older versions of MySQL and reportedly SQLITE (although in my testing it it's not permitted), it seldom makes sense to have a null primary key since only one such record is permitted.

[MasterOdin]

Alright, cool, that solves that. Identity columns are not null by default, all other columns are null by default. Should be easy enough to do, and would suggest landing onto the 0.13 release given its BC breaking.

[Bilge]

Just before we go rushing into this, to play Devil's advocate, are we sure SQL parity is a good reason for changing this default in the Phinx API? I'm not sure Phinx defaults absolutely have to mirror SQL defaults, especially if one could argue for changing the default. For example, one might argue in good database design there should not be nullable columns, and this is actually the sentiment expressed in the Postgres manual:

[MasterOdin]

Philosophically, I actually like that phinx defaults to NOT NULL as I believe that should be the default on column creation. It saves me a number of keystrokes, and it enforces a "make it easy to follow best practices" model. However, it then violates the principle of least surprise given that both the SQL standard, and other tools (at least in my experience with the handful I've used) where NULL is the default, and you have to explicitly mark something as NOT NULL if you want that.

Given the creation of this issue and https://github.com/cakephp/phinx/pull/1845#issuecomment-664628306, I would lean on fixing for the principle of least surprise here, especially to make it easier for people who might be switching tools, or just thinking of how the phinx migrations translates to raw SQL and not being particularly well versed on the phinx docs.

[othercorey]

I would suggest that phinx be uniform in its defaults across DBs except where db-specific types are involved (like default timestamp precision).

[MasterOdin]

@dereuromark this can be closed with #1872 having been merged

This will be available in 0.13