-
-
Notifications
You must be signed in to change notification settings - Fork 529
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 default value for modManagerLog.occurred column #16526
Conversation
Add right default value for datatime-type column in modx_manager_log table Re-up of modxcms#15736 with migration Satisfy phpcs
It is nice to have NULL available in datetime columns. This is allowed with NO_ZERO_DATE and NO_ZERO_IN_DATE and in strict mode. Only MODX has some datetime columns with NULL allowed i.e. in the editedon column of the system settings. The following sql commands are working fine here to solve some similar issues in other MODX datebase columns. The temporary date changes are maybe not 100% correct but normally a date in the 1970s does not exist in those columns in MODX.
|
While this may tangentially be related to this PR, this proposal belongs in a separate issue or should be prepared in a separate PR. The modManagerLog.occurred column specifically should NOT allow NULL values. |
The issue with the migration script can be invalid existing values in the column, if I am right. In that case the database column definition can't be changed and an SQL error occurs. |
I believe the only issue was that the table could not be created in a system with these strict_mode options enabled. I don't believe there are cases of actual invalid values being inserted into the table, are there? If there are, we can certainly attempt to resolve them first, but I don't think this is an actual issue. |
According to the docs https://dev.mysql.com/doc/refman/8.0/en/sql-mode.html#sqlmode_no_zero_date only In this table it does not make sense to have a NULL value available. So I am fine with the patch. |
### What does it do? Adds a default value for the datetime column in the modx_manager_log table that is compatible with strict modes, which may be enabled in some environments. Re-up of #15736 with migration ### Why is it needed? Beginning with MySQL > 5.7.8 added strict modes ERROR_FOR_DIVISION_BY_ZERO, NO_ZERO_DATE, NO_ZERO_IN_DATE. With these strict modes enabled, a datetime default value cannot be NULL and must be > '0000-00-00'. ### How to test Install/update on MySQL 5.7.8 ### Related issue(s)/PR(s) Re-up of #15736 Backport of #16526
### What does it do? Adds a default value for the datetime column in the modx_manager_log table that is compatible with strict modes, which may be enabled in some environments. Re-up of #15736 with migration Re-up of #16520 to target 3.0.x ### Why is it needed? Beginning with MySQL > 5.7.8 added strict modes ERROR_FOR_DIVISION_BY_ZERO, NO_ZERO_DATE, NO_ZERO_IN_DATE. With these strict modes enabled, a datetime default value cannot be NULL and must be > '0000-00-00'. ### How to test Install/update on MySQL 5.7.8 ### Related issue(s)/PR(s) Re-up of #15736 Re-up of #16520 --------- Co-authored-by: Mark Hamstra <[email protected]>
What does it do?
Adds a default value for the datetime column in the modx_manager_log table that is compatible with strict modes, which may be enabled in some environments.
Re-up of #15736 with migration
Re-up of #16520 to target 3.0.x
Why is it needed?
Beginning with MySQL > 5.7.8 added strict modes ERROR_FOR_DIVISION_BY_ZERO, NO_ZERO_DATE, NO_ZERO_IN_DATE. With these strict modes enabled, a datetime default value cannot be NULL and must be > '0000-00-00'.
How to test
Install/update on MySQL 5.7.8
Related issue(s)/PR(s)
Re-up of #15736
Re-up of #16520