Fix ORDER BY clauses being reset by count_all_results()

[pgee70]

recent changes broke query ordering giving a warning in line 1238 = '$this->qb_cache_orderby = array_merge($this->qb_cache_orderby, $qb_orderby);'
the warning message was about array_merge not working with a NULL object.

this was because on subsequent re-queries, the qb_cache_orderby was set to NULL not Array() as it is should have been. See the initial instantiation of the $this->qb_cache_orderby
This PR should fix it.

[narfbg]

The "recent changes" in question is the patch for #5168.

But while I can confirm that this is a bug, the warning itself is only a symptom. What's the actual effect that this has on queries? How do they break?

[pgee70]

This came up as a warning in my unit tests - I didn’t notice this affecting anything on prod. But I use query caching to help with data tables. if you array merge with null the output will be null. So: php > $a = null; php > $b = [1,2,3]; php > $c = array_merge($a,$b); PHP Warning: array_merge(): Argument #1 is not an array in php shell code on line 1 PHP Stack trace: PHP 1. {main}() php shell code:0 PHP 2. array_merge() php shell code:1 Warning: array_merge(): Argument #1 is not an array in php shell code on line 1 Call Stack: 62.1301 351296 1. {main}() php shell code:0 62.1301 351296 2. array_merge() php shell code:1 php > var_dump($c); php shell code:1: NULL So, this means that the orders already set would be lost on subsequent orders, after that buggy line was executed.

…

On 3 Oct 2017, at 8:05 pm, Andrey Andreev ***@***.***> wrote: The "recent changes" in question is the patch for #5168 <#5168>. But while I can confirm that this is a bug, the warning itself is only a symptom. What's the actual effect that this has on queries? How do they break? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5282 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFqfHgFcDB9zTXA0yFrY9L-qKBo9G7dRks5sofjGgaJpZM4PrYDt>.

[narfbg]

Ok, thanks!

Changed the title to reflect what the PR is actually about. As a side note, please target the develop branch unless you know that something is only relevant to 3.1-stable. We do backport all PR patches manually.