- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.7k
[4.0] [com_newsfeeds] Fix default value for not nullable datetime column #26380
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
          
     Merged
      
      
            wilsonge
  merged 12 commits into
  joomla:4.0-dev
from
richard67:4.0-dev-datetime-default-com-newsfeeds
  
      
      
   
  Oct 7, 2019 
      
    
                
     Merged
            
            [4.0] [com_newsfeeds] Fix default value for not nullable datetime column #26380
                    wilsonge
  merged 12 commits into
  joomla:4.0-dev
from
richard67:4.0-dev-datetime-default-com-newsfeeds
  
      
      
   
  Oct 7, 2019 
              
            Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    | Please wait with testing until PR #26295 is merged. | 
              
                    wilsonge
  
              
              approved these changes
              
                  
                    Sep 28, 2019 
                  
              
              
            
            
              
                    wilsonge
  
              
              approved these changes
              
                  
                    Oct 3, 2019 
                  
              
              
            
            
              
                    Quy
  
              
              reviewed
              
                  
                    Oct 3, 2019 
                  
              
              
            
            
        
          
                administrator/components/com_admin/sql/updates/mysql/4.0.0-2019-06-29.sql
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …-default-com-newsfeeds
| Ready for test now as PR #26295 has been merged. | 
| Thanks! | 
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
Pull Request for Issue #24535 (part).
Summary of Changes
Continuation of Pull Request (PR) #25362 .
This PR fixes all datetime columns of the
com_newsfeedsdatabase table#__newsfeedsso there will not be anyInvalid value '0000-00-00 00:00:00' for datetimeerror anymore on MySQL 5.7 or later when strict mode is enabled.Currently this table only contains 2 not nullable datetime columns,
createdandmodified. Those will be handled like the same columns of the#__bannerstable in PR #26372 , i.e. they will get no default value.The default value is only used when inserting new records without specifying values for that particular column. Not having a default 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.
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 particula table into 1 single
ALTER TABLEstatement 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
configuration.phpand delete all Joomla database tables in PhpMyAdmin or PhpPgAdmin (depending on your database type).datetime/timestamp without timezonecolumns.Result: See section "Expected result" below.
Test 2: Update sql script
installation/sql/mysql/joomla.sqlinto the SQL command window but don't execute the commands yet:This switches off strict mode to the SQL will run on MySQL 5.7 or later.
administrator/components/com_admin/sql/updates/mysql/4.0.0-2019-06-29.sqloradministrator/components/com_admin/sql/updates/postgresql/4.0.0-2019-06-29.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.#__by your database prefix in the SQL statements pasted before in the SQL input window.datetime/timestamp without timezonecolumns.Result: See section "Expected result" below.
Expected result
Newsfeeds work as well as before. In database there are no columns of type
datetimehaving value '0000-00-00 00:00:00' in a MySQL database, and there is no invalid default value anymore in MySQL >= 5.7 with strict mode on in that kind of database.There is one exception: When checking in a newsfeed using com_checkin, the
checked_out_timeis 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. therechecked_out_timeis set to NULL.Actual result
Newsfeeds work. In a MySQL database there might be columns of type
datetimehaving value '0000-00-00 00:00:00', and the default value of these database columns is invalid in MySQL >= 5.7 with strict mode on.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.