Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 4 years ago

#26847 closed defect (bug) (fixed)

MySQL 5.6 default install can break WordPress

Reported by: pento's profile pento Owned by: nacin's profile nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 2.7
Component: Database Keywords: has-patch commit
Focuses: Cc:

Description

As of MySQL 5.6, the sql_mode STRICT_TRANS_TABLES is defined by default. This means that for transactional tables (ie, InnoDB), NO_ZERO_DATE is implied.

Related:

#8857
#16821 (For which I just discovered I downvoted a similar solution. gg, me.)

Attachments (7)

26847.diff (1.3 KB) - added by pento 10 years ago.
26847.2.diff (1.7 KB) - added by pento 10 years ago.
26847.3.diff (1.8 KB) - added by pento 10 years ago.
26847.4.diff (1.8 KB) - added by pento 10 years ago.
26847.5.diff (4.5 KB) - added by pento 10 years ago.
26847.6.diff (4.9 KB) - added by pento 10 years ago.
26847.7.diff (4.7 KB) - added by DrewAPicture 10 years ago.
inline docs tweaks

Download all attachments as: .zip

Change History (21)

@pento
10 years ago

#1 @pento
10 years ago

To expand on my comment in #16821, setting the sql_mode when in WordPress is a solution involving changing the server settings, which I wasn't wild about.

On further consideration, our other (actually viable) options would be:

  • Rewrite INSERT queries to INSERT IGNORE, which has the terrible side effect of ignoring warnings on other columns, not just the DATETIME columns.
  • Rewriting all queries so that write queries have zero dates converted to null, and read queries have null dates converted to zero, effectively adding a zero date compatibility layer.

Both of these options have the down sides of hideous regexp, and they won't fix queries not run through WPDB.

So, sql_mode isn't amazing, but it's the best option for fixing a bug that we actually need to pay attention to now.

#2 follow-up: @morgantocker
10 years ago

It might be worth adding ONLY_FULL_GROUP_BY to the list of unsupported sql-modes. I am running with it on my blog, and receiving some errors:

[Mon Jan 27 12:27:56 2014] [error] [client 99.232.156.151] WordPress database error 'wordpress_tockerca.wp_posts.post_date' isn't in GROUP BY for query SELECT YEAR(post_date) AS year, MONTH(post_date) AS month, count(ID) as posts FROM wp_posts WHERE post_type = 'post' AND post_status = 'publish' GROUP BY YEAR(post_date), MONTH(post_date) ORDER BY post_date DESC made by require('wp-blog-header.php'), require_once('wp-includes/template-loader.php'), include('/themes/twentytwelve/single.php'), get_sidebar, locate_template, load_template, require_once('/themes/twentytwelve/sidebar.php'), dynamic_sidebar, call_user_func_array, WP_Widget->display_callback, WP_Widget_Archives->widget, wp_get_archives, referer: http://www.tocker.ca/categories/books

[Mon Jan 27 12:29:18 2014] [error] [client 99.232.156.151] WordPress database error Mixing of GROUP columns (MIN(),MAX(),COUNT(),...) with no GROUP columns is illegal if there is no GROUP BY clause for query SELECT COUNT(*) FROM wp_comments WHERE ( comment_approved = '0' OR comment_approved = '1' ) AND comment_post_ID = 143 ORDER BY comment_date_gmt DESC LIMIT 1 made by include('wp-admin/edit-form-advanced.php'), do_meta_boxes, call_user_func, post_comment_meta_box, get_comments, WP_Comment_Query->query, referer: http://www.tocker.ca/wp-admin/revision.php?revision=146

Last edited 10 years ago by morgantocker (previous) (diff)

@pento
10 years ago

#3 in reply to: ↑ 2 @pento
10 years ago

Replying to morgantocker:

It might be worth adding ONLY_FULL_GROUP_BY to the list of unsupported sql-modes.

Nice catch! I've added that in attachment:26847.2.diff, as well as a little bit of code shuffling.

Unrelated, I'm wondering if it's worth having a filter on the SQL mode array, so plugins can modify the list if they need to.

#4 @morgantocker
10 years ago

I think that would be nice, particularly if as part of that there is a developer-mode friendly way of disabling this workaround and running MySQL in a strict way. This will help advanced users more easily find truncation bugs etc, and could be used by CI tools as well.

@pento
10 years ago

@pento
10 years ago

#5 @pento
10 years ago

attachment:26847.4.diff​ adds a filter incompatible_sql_modes.

(attachment:26847.3.diff​ had the wrong filter name.)

@pento
10 years ago

#6 @pento
10 years ago

attachment:26847.5.diff

I realised the filter won't actually work, the DB connect happens way before plugins can add a filter. So, this patch turns set_sql_mode() into a function that a plugin can call at a later date, if they need to add or remove their own SQL modes.

The patch now includes unit tests.

#7 @nacin
10 years ago

#24912 was marked as a duplicate.

#8 @WebDragon
10 years ago

just noting that #24912 was not an exact duplicate, but would allow WP to function on servers with SQL_mode = ANSI,TRADITIONAL,ONLY_FULL_GROUP_BY until the team is actually able to fully patch wordpress to function in a strict MySQL environment.

And it's a one-line fix.

I've been patching WP with this since submitting the original ticket, with no problems.

Version 0, edited 10 years ago by WebDragon (next)

@pento
10 years ago

#9 @pento
10 years ago

  • Keywords commit added; dev-feedback removed

attachment:26847.6.diff adds PHPDocs for the new filter.

Unless there's any more feedback on it, I'm happy with the state of this patch.

@DrewAPicture
10 years ago

inline docs tweaks

#10 @DrewAPicture
10 years ago

26847.7.diff just tweaks the inline docs a bit for the new property, method, and hook.

#11 @nacin
10 years ago

  • Owner set to nacin
  • Status changed from new to accepted

#12 @nacin
10 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 27072:

Ensure compatibility with MySQL 5.6 which has stricter SQL modes by default.

Disables NO_ZERO_DATE, ONLY_FULL_GROUP_BY, STRICT_TRANS_TABLES, STRICT_ALL_TABLES, TRADITIONAL. Introduces wpdb::set_sql_mode() with an incompatible_sql_modes filter so a plugin can alter the set mode after the fact.

props pento.
fixes #26847.

#13 @felixq
10 years ago

Changelog MySQL 5.6.17:

Incompatible Change: The deprecated ERROR_FOR_DIVISION_BY_ZERO, NO_ZERO_DATE, and NO_ZERO_IN_DATE SQL modes now do nothing. Instead, their previous effects are included in the effects of strict SQL mode (STRICT_ALL_TABLES or STRICT_TRANS_TABLES). In other words, strict mode now means the same thing as the previous meaning of strict mode plus the ERROR_FOR_DIVISION_BY_ZERO, NO_ZERO_DATE, and NO_ZERO_IN_DATE modes. This change reduces the number of SQL modes with an effect dependent on strict mode and makes them part of strict mode itself.

#14 @morgantocker
10 years ago

@felixq - Thanks for pasting. My thoughts are that this will only affect Wordpress if the minimum MySQL version is raised to 5.6.17. This probably will not happen for quite a few years.

Note: See TracTickets for help on using tickets.