addTimestamps: update updated_at timestamp on row update

[kevingentile]

Link #868 . Updates don't reflect in the updated_at column for the default convenience methods.

[kevingentile]

Looks like the SQLiteAdapter doesn't like the update syntax:

1) Test\Phinx\Db\Adapter\SQLiteAdapterTest::testAlterTableColumnAdd
PDOException: SQLSTATE[HY000]: General error: 1 near "UPDATE": syntax error
/home/travis/build/cakephp/phinx/src/Phinx/Db/Adapter/PdoAdapter.php:181
/home/travis/build/cakephp/phinx/src/Phinx/Db/Adapter/SQLiteAdapter.php:694
/home/travis/build/cakephp/phinx/src/Phinx/Db/Util/AlterInstructions.php:119
/home/travis/build/cakephp/phinx/src/Phinx/Db/Adapter/PdoAdapter.php:606
/home/travis/build/cakephp/phinx/src/Phinx/Db/Adapter/PdoAdapter.php:981
/home/travis/build/cakephp/phinx/src/Phinx/Db/Plan/Plan.php:154
/home/travis/build/cakephp/phinx/vendor/cakephp/collection/CollectionTrait.php:69
/home/travis/build/cakephp/phinx/src/Phinx/Db/Plan/Plan.php:155
/home/travis/build/cakephp/phinx/src/Phinx/Db/Table.php:723
/home/travis/build/cakephp/phinx/src/Phinx/Db/Table.php:629
/home/travis/build/cakephp/phinx/src/Phinx/Db/Table.php:688
/home/travis/build/cakephp/phinx/tests/Phinx/Db/Adapter/SQLiteAdapterTest.php:1152

[MasterOdin]

I will take a look later on today or tomorrow. SQLite is annoying with how it supports altering a table with some pretty awful restrictions that require cumbersome workarounds.

[kevingentile]

@MasterOdin cool! Thanks for checking. It looks like some level of abstraction around the Column would let you override getValidOptions so at least you can remove the "update" option and handle the exception.

[MasterOdin]

Ah, it looks like we should delete this check from the sqlite adapter:

phinx/src/Phinx/Db/Adapter/SQLiteAdapter.php

Lines 1483 to 1485 in 54ece1f

	if ($column->getUpdate()) {
	$def .= ' ON UPDATE ' . $column->getUpdate();
	}

SQLite does not support the ON UPDATE syntax (it's specific to mysql) and postgresql and sqlserver just ignore that value. It's possible to support a similar function within non-MySQL DBs, but it requires creating triggers which phinx does not have a good handle for dealing with automatically. I've created PR #1800 to clarify this as well as remove the offending lines from sqlite adapter.

---

This all does raise the point of whether or not this should be accepted as it does make the default behavior of one adapter (MySQL) different than the others as it's the only one that supports this clause.

[dereuromark]

Can we merge?

[MasterOdin]

Can we merge?

Assuming people are fine with MySQL having different behavior, yes? I can write up a PR adding a note about this to the documentation.

[dereuromark]

@lorenzo @markstory @othercorey any opionions?

[markstory]

I'm fine with MySQL being different. People using MySQL will want to be able to use ON UPDATE syntax.

[dereuromark]

We might want to further document this then, though. Merging for now.