Memory exhaustion caused by very many unattached media

[andy]

A blog with over 30,000 unattached images was exhausting available memory on unattached attachment pages. The exhaustion occurred in previous_image_link() -> adjacent_image_link() -> get_children() due to the large number of rows selected with post_parent=0.

This is not the only place where get_posts() might be called with post_parent=0. A similar query appears in twentythirteen_the_attached_image() but the effect is wisely mitigated by fields=ids. Earlier, twentyten/loop-attachment.php called get_children() with post_parent=0 on unattached image pages which is horribly inefficient when there are many unattached images.

Attached patch depends on #24687. The change to adjacent_image_link() creates a MySQL query which selects only the desired row. This fixes the memory exhaustion error on the blog with 30,000 unattached images. The once-only filters would be unnecessary if WP_Query accepted arbitrary extra WHERE strings.

[SergeyBiryukov]

Related: #23044

[SergeyBiryukov]

Related: [7222]

[wonderboymusic]

these are all marked 3.7-early

[ocean90]

Punting due to lack of testing.

[ericlewis]

Replying to andy:

The once-only filters would be unnecessary if WP_Query accepted arbitrary extra WHERE strings.

That's interesting. Are you subtly proposing this idea?

[kitchin]

About the patch
return "$where AND (...
should be
return "( $where ) AND (...
There are operators that bind less tightly, like OR.

Is that what you meant by testing? :)

[andy]

I think since #26042 is closed this ticket now just needs a new patch and testing.

[dllh]

Updated patch

[dllh]

I've updated the patch for trunk and given it a test drive. It seems to work well for me. Any chance of nudging this along?

[kitchin]

My objection above still stands, to $where without parentheses in return "$where AND ...

[iCaleb]

Found an issue with the query for retrieving the next attachment link. It will always link to the last image attached to the post, not necessarily the next one. Turns out, 'orderby' => "menu_order $order, ID $order" will result in ORDER BY wp_posts.menu_order DESC, wp_posts.ID DESC no matter what $order is (it'll never be ASC). This can be solved by instead passing an array into the orderby param as supported since WP 4.0.

---

Next up, array_pop was also causing a PHP notice:

Notice: Only variables should be passed by reference at wp-includes/media.php:2812

Line 2812 was 'post_parent' => $post->post_parent. This is solved by either passing a single variable into array_pop(), else avoiding using it. Since in this case get_children() is just either going to return a single element array or an empty array, I opted for just accessing the first key directly if it exists.

---

Lastly, I did apply the return "( $where ) AND ( changes mentioned above, but it results in a SQL syntax error.

[iCaleb]

Updated patch w/ previously mentioned changes.

[david.binda]

Adding basic unit tests for the previous_image_link() and next_image_link() functions.

[david.binda]

My objection above still stands, to $where without parentheses in return "$where AND ...

@kitchin , do you happen to have an example of code which would break by applying the patch the way it's being proposed here?

My understanding is that the WHERE clause in WP_Query::get_posts where posts_where filter is being applied is being built as a chain of AND clauses. In case a plugin would be filtering posts_where while adding OR w/o proper parentheses, it would either have to be doing some weird string modifications (as the $where passed to the callback starts with AND, eg.: AND wptests_posts.post_parent = 85 AND (wptests_posts.post_mime_type LIKE 'image/%') AND wptests_posts.post_type = 'attachment' AND ((wptests_posts.post_status = 'inherit')) and is being appended to ... WHERE 1=1 string, eg.: SELECT wptests_posts.ID FROM wptests_posts WHERE 1=1 ), or would be appending OR to the end of a string of ANDs effectively bypassing all the previous conditions.

That said, I don't think the callback proposed here should really wrap the initial part of the $where into parenthesis, especially as it would require us replacing the leading AND by AND ( which really feels wrong and error prone. In case something would get wrong by not using parenthesis there, it would be because some plugin code does it wrong.