? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
21 Sep 2019

Pull Request for Issue #24535 (part).

Summary of Changes

Continuation of Pull Request (PR) #25360 .

This PR fixes all datetime columns of com_banners tables so there will not be any Invalid value '0000-00-00 00:00:00' for datetime error anymore on MySQL 5.7 or later when strict mode is enabled, and the datetime (MySQL) or timestamp without time zone (PostgreSQL) columns will allow NULL values wherever useful/possible.

Change no. 1

This PR continues the work of PR #25360 by making following database columns nullable, too:

  • Table #__banners column reset
  • Table #__banner_clients column checked_out_time

Change no. 2

In addition this PR removes the default values from columns created and modified of table #__banners. This will enforce to insert new records with values for these columns being provided and throw an SQL error if some of these values is not specified, i.e. such errors will not be hidden anymore.

Old data will be updated as little as possible. The created column will be not touched at all. We can assume that for core components there is no data with values '0000-00-00 00:00:00' on MySQL or '1970-01-01 00:00:00' on PostgreSQL, and data created by 3rd party components should not be modified. The modified column will be set to the value of the created column only has value '0000-00-00 00:00:00' on MySQL or '1970-01-01 00:00:00' on '1970-01-01 00:00:00'. Our PHP already sets the modified time to the created time when saving new records, i.e. modified = created means never modified.

Change no. 3

This PR fixes corresponding PHP so for the nullable columns there is no check anymore for pseudo null dates or abused oldest date null dates but only for real NULL values, which is safe when we have made sure with the sql update script that on an updated core there is none of these pseudo null dates or abused oldest date null dates.

Change no. 4

Finally, this PR updates the component-specific install script install.mysql.utf8.sql, which might be used for a discovery installation of com_banners` in case if that's needed for some reason. Not use if that has to be done, it has not been done with PR #25360 before.

Note on updating old 4.0 sql update scripts

Since we are not in Beta phase yet and so don't have to support updates from 4.0-Alpha-x to 4.0-Alpha-y or between nightly builds, we can change the existing 4.0 update scripts (but of course not pre-4.0 scripts). Later when in beta this will not be allowed anymore because we have to support updating from Beta-x to Beta-y (with y > x of course), so now is a good time to fix them.

Furthermore it makes sense to keep the schema updates for the datetime columns of each table together in one script, because on MySQL it might be necessary to combine all changes for a particular table into 1 single ALTER TABLE statement in order to run properly in strict mode (i.e. strict tables enabled). Currently the db checker/fixer can't understand such statements, but it might be necessary to teach that tool soon to do that.

That's why this PR updates the existing sql update script and doesn't create a new one.

Testing Instructions

Testers please report back the database kind (MySQL or PostgreSQL) on which you have tested.

If you have both MySQL and PostgreSQL, please test on both if possible.

Test 1: New installation

  1. Apply the patch on a clean 4.0-dev branch using git or merging manually, or apply it on an installation of current 4.0-dev using patchtester.
  2. If used patchtester on an existing installation in step 1, delete file configuration.php and delete all Joomla database tables in PhpMyAdmin or PhpPgAdmin (depending on your database type).
  3. Do a new installation, login to backend, confirm the statistics dialog, go to global config and set error reporting to maximum or development in server settings.
  4. Play around with banners, create some, publish some, make them be tracked.
  5. After any action, check in your database the relevant datetime/timestamp without timezone columns.

Result: See section "Expected result" below.

Test 2: Update sql script

  1. Install a clean clean 4.0-dev, login to backend, confirm the statistics dialog, go to global config and set error reporting to maximum or development in server settings.
  2. Apply the changes from this PR e.g. manually or with patch tester.
  3. Open PhpMyAdmin or PhpPgAdmin (depending on your database type), select your database and then go to the SQL command input.
  4. On MySQL copy the first line of file installation/sql/mysql/joomla.sql into the SQL command window but don't execute the commands yet:
SET SQL_MODE = "NO_AUTO_VALUE_ON_ZERO";

This switches off strict mode to the SQL will run on MySQL 5.7 or later.

  1. Copy the content of file administrator/components/com_admin/sql/updates/mysql/4.0.0-2019-06-28.sql or administrator/components/com_admin/sql/updates/postgresql/4.0.0-2019-06-28.sql (depending on your database type) into the SQL command window, in case of MySQL below the previously pasted commands, but don't execute the commands yet.
  2. Replace #__ by your database prefix in the SQL statements pasted before in the SQL input window.
  3. Put the cursor to the beginning of the 1st SQL statement in the SQL input window and now execute all SQL commands.
  4. Play around with banners, create some, publish some, make them be tracked.
  5. After any action, check in your database the relevant datetime/timestamp without timezone columns.

Result: See section "Expected result" below.

Expected result

Banners work as well as before. In a MySQL database there are no columns of type datetime having value '0000-00-00 00:00:00'. Nullable columns have NULL when they should have in any kind of database. Following columns are nullable, too, beside those which were already before this PR:

  • Table #__banners column reset
  • Table #__banner_clients column checked_out_time

There is one exception: When checking in a banner using com_checkin, the checked_out_time is set to '0000-00-00 00:00:00' on MySQL and '1970-01-01 00:00:00' on PosgreSQL. This will be changed with a separate, future PR for com_checkin. Checking in an item with the lock icon in list display works, i.e. there checked_out_time is set to NULL.

Actual result

Banners work. In a MySQL database there might be are columns of type datetime having value '0000-00-00 00:00:00'. In any kind of database nullable columns don't always have NULL when they should have.

Documentation Changes Required

Maybe core developer docs and extension developer docs should be updated to encourage them not to use '0000-00-00 00:00:00' on MySQL anymore but use real NULL and not abuse '1970-01-01 00:00:00' on PostgreSQL as a speudo null date anymore and use real NULL values also there.

Votes

# of Users Experiencing Issue
0/1
Average Importance Score
3.00

avatar richard67 richard67 - open - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 21 Sep 2019
Category SQL Administration com_admin Postgresql com_banners Front End Installation
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
Title
[4.0] [WiP] [com_banners] Fix default values of datetime columns
[4.0] [WiP] [com_banners] Nullable datetime columns and fix default values not nullable datetime columns
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
Title
[4.0] [WiP] [com_banners] Nullable datetime columns and fix default values not nullable datetime columns
[4.0] [WiP] [com_banners] Finish nullable and fix default values for not nullable datetime columns
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
Title
[4.0] [WiP] [com_banners] Finish nullable and fix default values for not nullable datetime columns
[4.0] [com_banners] Finish nullable and fix default values for not nullable datetime columns
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67
richard67 - comment - 21 Sep 2019

@HLeithner Could you request a review by @wilsonge for me? I don't have the privilege to request reviews with GitHub methods in this repo, or I am too silly.

avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67 richard67 - change - 21 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 21 Sep 2019
avatar richard67
richard67 - comment - 21 Sep 2019

@alikon Could you test?

avatar alikon
alikon - comment - 22 Sep 2019

i've played a bit with it on PostgreSQL 11.5 - linux
NULL are managed well

avatar alikon alikon - test_item - 22 Sep 2019 - Tested successfully
avatar alikon
alikon - comment - 22 Sep 2019

I have tested this item successfully on 3274488


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26372.

avatar richard67 richard67 - change - 22 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 22 Sep 2019
avatar richard67 richard67 - change - 22 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 22 Sep 2019
avatar richard67 richard67 - change - 24 Sep 2019
Labels Added: ?
avatar richard67
richard67 - comment - 27 Sep 2019

Please wait with further testing until PR #26295 is merged.

avatar richard67 richard67 - change - 28 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 28 Sep 2019
avatar richard67 richard67 - change - 28 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 28 Sep 2019
avatar richard67 richard67 - change - 28 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 28 Sep 2019
avatar richard67 richard67 - change - 29 Sep 2019
The description was changed
avatar richard67 richard67 - edited - 29 Sep 2019
avatar richard67 richard67 - change - 3 Oct 2019
The description was changed
avatar richard67 richard67 - edited - 3 Oct 2019
avatar richard67
richard67 - comment - 6 Oct 2019

Ready for test now as PR #26295 has been merged.

avatar richard67 richard67 - change - 6 Oct 2019
The description was changed
avatar richard67 richard67 - edited - 6 Oct 2019
avatar richard67 richard67 - change - 6 Oct 2019
The description was changed
avatar richard67 richard67 - edited - 6 Oct 2019
avatar richard67 richard67 - change - 6 Oct 2019
The description was changed
avatar richard67 richard67 - edited - 6 Oct 2019
avatar richard67 richard67 - change - 6 Oct 2019
The description was changed
avatar richard67 richard67 - edited - 6 Oct 2019
avatar richard67
richard67 - comment - 6 Oct 2019

@alikon Could you repeat your test? There have been made a few changes since then.

avatar alikon alikon - test_item - 7 Oct 2019 - Tested successfully
avatar alikon
alikon - comment - 7 Oct 2019

I have tested this item successfully on 12a3a2d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26372.

avatar wilsonge wilsonge - change - 7 Oct 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-10-07 23:00:48
Closed_By wilsonge
avatar wilsonge wilsonge - close - 7 Oct 2019
avatar wilsonge wilsonge - merge - 7 Oct 2019
avatar wilsonge
wilsonge - comment - 7 Oct 2019

Thanks!

Add a Comment

Login with GitHub to post a comment