Easy-digital-downloads: 3.0 - Default datetime value `0000-00-00 00:00:00` causes errors in MySQL 5.7+

[ashleyfae]

Enhancement Request

All the datetime default values are set to 0000-00-00 00:00:00. I propose we use null instead (or in some cases, like date_created we could use CURRENT_TIMESTAMP).

Justification or use case

MySQL version 5.7 defaults to disallowing 0000-00-00 00:00:00 as a default value unless you override the settings to explicitly allow it.

https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-strict

Strict mode affects whether the server permits '0000-00-00' as a valid date:

If strict mode is not enabled, '0000-00-00' is permitted and inserts produce no warning.

If strict mode is enabled, '0000-00-00' is not permitted and inserts produce an error, unless IGNORE is given as well. For INSERT IGNORE and UPDATE IGNORE, '0000-00-00' is permitted and inserts produce a warning.

RCP has been using 0000-00-00 00:00:00 for default values too. Recently Mihai was trying to drop a column from inside PhpMyAdmin to do some RCP testing, and he wasn't able to do so because a different column in the same table had a default value of 0000-00-00 00:00:00 https://github.com/restrictcontentpro/restrict-content-pro/issues/2698#issuecomment-584580987

SQL query:
ALTER TABLE `wp_rcp_discounts` DROP `one_time`
MySQL said: Documentation

#1067 - Invalid default value for 'expiration'

[cklosowski]

I would agree with the above suggestion to use 'null' as they entry when not needed, or CURRENT_TIMESTAMP as the default for things like DATE_CREATED or when updating any records.

[ashleyfae]

Related, if we go ahead with this: https://github.com/berlindb/core/issues/22

[spencerfinnell]

@nosegraze is this a blocker for #7581? Never mind. Related but not a blocker.

[ashleyfae]

My PR for this will also be fixing a bug @spencerfinnell found that sets the order date to something wonky when in the Add New Order UI:

[cklosowski]

Code looks good, but I’m going to have to spend some time running this through the paces for sure.

We should make sure to test this on both < 5.7 installs as well as 5.7 installs to make sure we don’t unintentionally break it the other direction.

[JJJ]

Just seeing this now. Are we sure that CURRENT_TIMESTAMP is correct for a datetime column type? I think that might be more for the timestamp column type, but I'm not 100% positive.

Also, WordPress does make sure that NO_ZERO_DATE is listed as an incompatible MySQL mode, so zero datetime values should be fine for a little while longer, though I do agree it's a good idea to phase them out.

cc @robincornett who was seeing MySQL errors from CURRENT_TIMESTAMP.

[ashleyfae]

@JJJ I think CURRENT_TIMESTAMP is valid but it looks like it might only be valid in MySQL 5.6.5+?

https://dev.mysql.com/doc/refman/5.6/en/timestamp-initialization.html

[robincornett]

I'm running MySQL 8.0.16 and ran into this error today:

[08-May-2020 13:48:51 UTC] WordPress database error You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CURRENT_TIMESTAMP' at line 1 for query ALTER TABLE mce_edd_customers ALTER COLUMNdate_createdSET DEFAULT CURRENT_TIMESTAMP made by require_once('wp-admin/admin.php'), do_action('admin_init'), WP_Hook->do_action, WP_Hook->apply_filters, EDD\Database\Tables\Customers->maybe_upgrade

@ashleyfae suggested changing line 105 to this:

$this->get_db()->query( "ALTER TABLE {$this->table_name} MODIFY `date_created` datetime NOT NULL default CURRENT_TIMESTAMP" );

and after dropping tables and starting fresh with that, I did not get the error.

I dropped tables again, reverted the line of code, and got the error again.