get_approved_comments() behaviour change with post_id=0

[dd32]

In [30098] get_approved_comments() was made a wrapper of WP_Comment_Query, this has had a slight backwards compat change.

Previously, $post_id was a required parameter, and if passed a falsey value would simply return an empty array (as it'd query for post_id = 0). After the change, it now queries for comments on ALL posts, this can very easily exhaust system resources if someone was calling the function incorrectly.

Attached is a patch for review, It simply makes $post_id a required parameter, and if it evaluates false the function returns an empty array.

[boonebgorges]

Thanks for the patch, dd32, and sorry for breaking stuff :) I wanted to verify that returning all comments when passing post_id=0 to WP_Comment_Query::query() was the expected behavior in 4.0 and not a regression (it was, and it's not). So the special case in get_approved_comments() looks like the right fix to me. 30412.patch​ contains unit tests for passing 0 to get_approved_comments() as well as WP_Comment_Query.

I'm posting this here rather than committing it straight away because I wanted to ask you about making $post_id required. In a quick search on Github, I couldn't find any instances of get_approved_comments() being called without a parameter, but it could be happening somewhere, so I don't see a reason why we should throw a notice in that case. At least, it's not necessary to fix the bug reported in this ticket.

[dd32]

I couldn't find any instances of get_approved_comments() being called without a parameter, but it could be happening somewhere, so I don't see a reason why we should throw a notice in that case.

Previously the parameter was required & used within the SQL, since the function will now return early if it's not specified, it makes sense to restore that behaviour.

[boonebgorges]

Previously the parameter was required

*headdesk*

[boonebgorges]

In 30402:

Return an empty array from get_approved_comments() when $post_id is empty.

This behavior was broken when moving the internals to WP_Comment_Query in
[30098]. As a result, get_approved_comments( 0 ) was fetching *all* approved
comments, causing performance issues.

Props dd32.
Fixes #30412.