ruckus / ruckusing-migrations

Database migrations for PHP ala ActiveRecord Migrations with support for MySQL, Postgres, SQLite
Other
506 stars 95 forks source link

Remove MySQL default value for timestamp #135

Open larsnystrom opened 10 years ago

larsnystrom commented 10 years ago

I just noticed that one of my timestamp columns for a table created with ruckusing was set to default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP, even though I never specified that through ruckusing. I then found what was going on in the MySQL Manual:

With neither DEFAULT nor ON UPDATE clauses, it is the same as DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP.

Personally I think this is strange behaviour, and I guess this doesn't apply to Postegres or Sqlite. If that's the case I suggest that ruckusing overrides this behaviour so that a timestamp column gets the same default value no matter what database you are using.

ruckus commented 10 years ago

That MySQL timestamp handling is a mess. I'm not even sure what we can do. Mainly because MySQL treats different timestamps columns in the same table, e.g. the 1st instance will be automatically updated, but subsequent ones are not.

This seems like a mess and for something for ruckusing to not get involved in.

I just checked out the source for ActiveRecord 4.1.1 and they seem to skirt the whole issue by just unsupporting timestamp as a whole and telling users to use datetime and transparently re-writing timestamp definitions to datetime:

https://github.com/rails/rails/commit/d0f8c46e1962b28d77209f367f12c2d2c77f4b12

Do you have any recommendations on how this should all be handled?

On Jun 26, 2014, at 7:02 AM, Lars Nyström notifications@github.com wrote:

I just noticed that one of my timestamp columns for a table created with ruckusing was set to default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP, even though I never specified that through ruckusing. I then found what was going on in the MySQL Manual:

With neither DEFAULT nor ON UPDATE clauses, it is the same as DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP. Personally I think this is strange behaviour, and I guess this doesn't apply to Postegres or Sqlite. If that's the case I suggest that ruckusing overrides this behaviour so that a timestamp column gets the same default value no matter what database you are using.

— Reply to this email directly or view it on GitHub.

larsnystrom commented 10 years ago

Summer vacation is over, so I guess it's time to answer here :)

Rails has the advantage of controlling the entire application stack and can therefore deal with timezone conversions and such at the application level transparently. This is not possible with ruckusing.

I'm mentioning that because timestamps are converted to UTC when being stored and then converted back to the timezone of the connection when retrieved. That's a wtf in itself, even though I guess there's some very bright idea behind it.

When you google for the difference between timestamps and datetimes people tend to press the fact that timestamps represent a specific point in time, whereas a datetime represents a time like it's being displayed on a clock. That's a pretty weird explanation, but it makes more sense when you consider the fact that datetimes doesn't have a timezone component (a timestamp does implicitly, since you know it's UTC).

However, since we can't control the way an application handles timezones through ruckusing this whole problem is left as an exercise for each developer to tackle. This means that there's really not much of a difference between timestamps and datetimes, besides date ranges.

Still, I guess it's better to offer the possibility of using either of them with Ruckusing. (this answer is getting long now, I know).

However, I still think that Ruckusing should define sane defaults for each column even though the developer leaves them out. When migrating from an RDBMS to another you would expect the migrations to produce equivalent schemas. Because Ruckusing is supposed to be database agnostic (at least that's my understanding of the project goal).

So I'm still for overriding MySQLs defaults in this case.

ruckus commented 10 years ago

Thanks for the explanation. At this point I'm kind of confused as to what the solution should be. I'm reading this thread and its just a blob of "bleh" in my head.

If I understand correctly, for MySQL when a timestamp column is specified AND there is no 'default' => 'something' specified in the extra options array for that column definition than we should over-ride the SQL to ultimately be:

DEFAULT 0

?

E.g.

$t->column('created_at', 'timestamp')

Right now generates a definition like:

created_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP

But it should be over-ridden as in CREATE TABLE foobars (created_at timestamp DEFAULT 0)

which MySQL ultimately generates:

created_at timestamp NOT NULL DEFAULT '0000-00-00 00:00:00'

Is that what you're saying?

Here is my MySQL command line exploration:

mysql> create table foos (created_at timestamp);
Query OK, 0 rows affected (0.02 sec)

mysql> show create table foos;
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                                         |
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------+
| foos  | CREATE TABLE `foos` (
  `created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
) ENGINE=InnoDB DEFAULT CHARSET=latin1 |
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

mysql> create table bars (created_at timestamp default 0);
Query OK, 0 rows affected (0.01 sec)

mysql> show create table bars;
+-------+------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                 |
+-------+------------------------------------------------------------------------------------------------------------------------------+
| bars  | CREATE TABLE `bars` (
  `created_at` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00'
) ENGINE=InnoDB DEFAULT CHARSET=latin1 |
+-------+------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)
larsnystrom commented 10 years ago

I would prefer that. I think it's nice to provide sane defaults where the developer omitted them.

The idea is to create the equivalent schemas on all different dbms if running the same method with ruckusing. In this case that requires us to override MySQL's defaults and provide our own. 

The alternative would be to override both SQLite and Postgres defaults to conform to MySQL's defaults. But I prefer the former option.

Of course you could also close this ticket if you think this is outside of what ruckusing is actually about. The feature we're talking about is not about migrations between schemas, but between databases.

On Fri, Sep 26, 2014 at 5:40 PM, Cody Caughlan notifications@github.com wrote:

Thanks for the explanation. At this point I'm kind of confused as to what the solution should be. I'm reading this thread and its just a blob of "bleh" in my head. If I understand correctly, for MySQL when a timestamp column is specified AND there is no 'default' => 'something' specified in the extra options array for that column definition than we should over-ride the SQL to ultimately be: DEFAULT 0 ? E.g. $t->column('created_at', 'timestamp') Right now generates a definition like: created_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP But it should be over-ridden as in CREATE TABLE foobars (created_at timestamp DEFAULT 0) which MySQL ultimately generates: created_at timestamp NOT NULL DEFAULT '0000-00-00 00:00:00' Is that what you're saying? Here is my MySQL command line exploration:

mysql> create table foos (created_at timestamp);
Query OK, 0 rows affected (0.02 sec)
mysql> show create table foos;
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                                         |
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------+
| foos  | CREATE TABLE `foos` (
  `created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
) ENGINE=InnoDB DEFAULT CHARSET=latin1 |
+-------+------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)
mysql> create table bars (created_at timestamp default 0);
Query OK, 0 rows affected (0.01 sec)
mysql> show create table bars;
+-------+------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                 |
+-------+------------------------------------------------------------------------------------------------------------------------------+
| bars  | CREATE TABLE `bars` (
  `created_at` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00'
) ENGINE=InnoDB DEFAULT CHARSET=latin1 |
+-------+------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

Reply to this email directly or view it on GitHub: https://github.com/ruckus/ruckusing-migrations/issues/135#issuecomment-56978879