Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix upgrade failures from zero value trxn_date #11745

Merged
merged 1 commit into from
Mar 5, 2018

Conversation

agh1
Copy link
Contributor

@agh1 agh1 commented Mar 1, 2018

Overview

Some longstanding CiviCRM installations would have upgrade failures going to 4.7.19 or higher with the database error,

Incorrect datetime value: '0000-00-00 00:00:00' for column 'trxn_date'

This change fixes those values to be NULL prior to the query that causes the problem.

Before

Certain sites with legacy data (canceled event registrations, potentially among other things, from around 2013) have trxn_date values of 0000-00-00 00:00:00 in the civicrm_financial_trxn table. This is not an allowable value for a datetime field, and an upgrade step on the way to 4.7.19 will cause the error,

ALTER TABLE civicrm_financial_trxn CHANGE pan_truncation pan_truncation VARCHAR( 4 ) NULL DEFAULT NULL COMMENT 'Last 4 digits of credit card' [nativecode=1292 ** Incorrect datetime value: '0000-00-00 00:00:00' for column 'trxn_date' at row XXX]

After

Before that query executes, all 0000-00-00 00:00:00 values in civicrm_financial_trxn.trxn_date are set to NULL. Meanwhile, those values seem to be always also occur in the receive_date field of the corresponding civicrm_contribution row. The upgrade now sets those values to NULL as well.

Technical Details

This shouldn't affect most users.

Comments

@KarinG @JoeMurray @eileenmcnaughton I suspect this is the legacy of some bug fixed in 4.3 or 4.4, but I couldn't easily find it.

Also, while I believe NULL is an improvement over 0000-00-00 00:00:00 since it's at least an acceptable value, those rows are now the only ones with null dates. Theoretically, this could cause problems in pages or functions that make use of the dates.

@eileenmcnaughton
Copy link
Contributor

test this please

1 similar comment
@seamuslee001
Copy link
Contributor

test this please

@tschuettler
Copy link
Contributor

@agh1 That may aswell been just some different server configuration, where those values had been permitted and used instead of NULL or the current time:
https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-strict

https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_explicit_defaults_for_timestamp

@KarinG
Copy link
Contributor

KarinG commented Mar 2, 2018

@agh1 - agreed - no data (NULL) is better than wrong data;

@eileenmcnaughton
Copy link
Contributor

I feel like this is a bit of an obscure scenario but those columns do ALLOW NULL and I feel this is obscure enough to defer to the person who has done the research. Merging

@eileenmcnaughton eileenmcnaughton merged commit 247e904 into civicrm:master Mar 5, 2018
@mlutfy mlutfy added this to the 4.7.32 milestone Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants