Fix MySQL default timestamp for 5.5.5+ bwc #59805
Conversation
We switched to using `CURRENT_TIMESTAMP` in #53940. Find details in commit 9606375. However, that caused backward compatibility issues with MySQL 5.5.5 since `DEFAULT CURRENT_TIMESTAMP` isn't supported. Since we started indexing on `date_created`, and index creation fails on '0000-00-00 00:00:00', we use '1970-01-01 00:00:00' which seems to be another commonly used placeholder for "no data". This makes MySQL 5.5.5+ happy, and it also makes the index happy.
📝 WalkthroughWalkthroughThe change updates the WooCommerce database schema generation to conditionally set the default value for the Changes
Sequence Diagram(s)sequenceDiagram
participant Installer as WC Installer
participant Database as Database
Installer->>Database: Check MySQL version
alt MySQL ≥ 5.6
Installer->>Database: Create wc_order_product_lookup with date_created default CURRENT_TIMESTAMP
else MySQL < 5.6
Installer->>Database: Create wc_order_product_lookup with date_created default '1970-01-01 00:00:00'
end
Database-->>Installer: Table created with appropriate default
Estimated code review effort2 (10–30 minutes) 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
There was a problem hiding this comment.
@bor0 since date_created is indexed, have you checked if this value is also being set when adding a new row or if it depends on the DEFAULT? I'm asking because if it's the latter, then there will be many rows with '1970-01-01 00:00:00' and index won't be as performant (which wouldn't be the case with CURRENT_TIMESTAMP)
|
@NeosinneR good point. The column's value seems to always be set here (didn't find other places where we insert), so it shouldn't be an issue. But, there are always customizations with hooks/filters, and to be more on the safe side, I guess we can still use |
There was a problem hiding this comment.
Looks good, thanks @bor0. I checked the wc_get_server_database_version() function and unless the $wpdb->get_var( 'SELECT VERSION()' ); fails (in which case the website has probably other issues), the 'number' should always be a string, so the version compare shouldn't fail.
|
Does the version check account for MariaDB usage? |
@nerrad it does. |
Changes proposed in this Pull Request:
We switched to using
CURRENT_TIMESTAMPin #53940. Find details in commit 9606375.However, that caused backward compatibility issues with MySQL 5.5.5 since
DEFAULT CURRENT_TIMESTAMPisn't supported.Since we started indexing on
date_created, and index creation fails on '0000-00-00 00:00:00', we use '1970-01-01 00:00:00' which seems to be another commonly used placeholder for "no data".This makes MySQL 5.5.5+ happy, and it also makes the index happy.
Closes pcShBQ-3tn#comment-5181.
How to test the changes in this Pull Request:
SHOW INDEX FROM wc_order_product_lookupshould show the index ondate_createdRepeat testing instructions with a more recent MySQL version.
Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment