[wp-trac] [WordPress Trac] #25775: WP_Date_Query table prefixing

[WordPress Trac]

#25775: WP_Date_Query table prefixing
-------------------------------------+------------------
 Reporter:  ew_holmes                |       Owner:
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  4.1
Component:  Query                    |     Version:  3.7
 Severity:  major                    |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+------------------
Changes (by boonebgorges):

 * keywords:  needs-refresh needs-unit-tests 4.1-early => has-patch needs-
     testing
 * milestone:  Future Release => 4.1


Comment:

 [attachment:25775.patch] is an attempt at fixing the problem that
 maintains backward compatibility with the filter and with existing use of
 `WP_Date_Query`. Much of the patch is making some minor alterations to
 existing unit tests so that they work with the modified
 `validate_column()`. Here's the executive summary:

 * When you pass a value to `validate_column()` that contains a dot, it's
 trusted - no validation takes place.
 * Return results of `validate_column()` are run through `esc_sql()`, since
 we're no longer so strictly checking against a whitelist (though tbh the
 check is not entirely strict at the moment).
 * When a non-dotted value is passed to `validate_column()`, check to see
 if it's a core date column (`$known_columns`), and if so, add the
 appropriate prefix.

 How this fixes the problem:

 * `test_date_query_with_taxonomy_join()` demonstrates the problem raised
 in the original ticket. Since no column is passed to the query,
 'post_date' is assumed; 'post_date' then validates to
 `$wpdb->posts.post_date`.
 * `test_validate_column_with_date_query_valid_columns_filter()` shows that
 the filter continues to work. If you are using this filter to add a custom
 column, it will continue to be validated. `validate_column()` won't add a
 table prefix, because it's not a "known column". But (a) this is not a
 regression - currently, if you add a custom column like this and then do a
 table join, you'll probably break something; and (b) devs using the filter
 should be encouraged to add table-prefixed columns to the list. (ie
 `$columns[] = 'my_table_name.my_column_name')

 Some of the changes suggested by wonderboymusic
 https://core.trac.wordpress.org/ticket/25775#comment:9 are interesting and
 worth pursuing (would you expect any less? <3 <3 <3) but are beyond the
 scope of this ticket. I also think his patch misunderstands what
 `WP_Comments_Query::$date_query` is supposed to do - we don't need to join
 against the posts table because we only care about the 'comment_date', as
 passed to `get_sql()` here
 https://core.trac.wordpress.org/browser/tags/4.0/src/wp-
 includes/comment.php#L441. (If his tests were converting `comment_date` to
 `post_date`, it must be because he had one of the other patches on this
 ticket applied to his build - they broke date queries for comments.)

 In the future might look for a more robust syntax for table-specific
 support in `WP_Date_Query`. See #29823. But that's not necessary to fix
 the problem at hand.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/25775#comment:31>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform