Sorting Bug / Wrong behavior?

php.internals

John Coggeshall

22 years ago
Consider the following: <?php $a = array('a', 'b', 'c', 'a'=>0, 'b'=>1, 'c'=>2); sort($a); print_r($a); ?> This produces a bogus output: Array ( [0] => a [1] => b [2] => 0 [3] => c [4] => 1 [5] => 2 ) Notice how 0 and c are switched incorrectly. Attached is a patch to zend_operators.c that fixes it.

Stefan Walk

22 years ago
On Tue, Jul 27, 2004 at 02:48:28PM -0500, John Coggeshall wrote:
> Notice how 0 and c are switched incorrectly. Attached is a patch to > zend_operators.c that fixes it.
The Manual says:
> Warning > > Be carefull when sorting arrays with mixed types values because sort() > can produce unpredictable results.
You could use SORT_STRING, natsort, etc... If this gets "fixed", the $a = "0d9"; var_dump(++$a, ++$a); thing could also be "fixed", as its WTF factor is as high... or "010" == "1e1" being true. It's all the consequence of PHPs handling with numbers and strings, which can produce results that strike you as odd.

Andi Gutmans

22 years ago
This isn't a bug. All those strings evaluate to 0. You should use a sort() function which is suitable. At 02:48 PM 7/27/2004 -0500, John Coggeshall wrote:

Marcus Börger

22 years ago
Hello John, Tuesday, July 27, 2004, 9:48:28 PM, you wrote:
> Consider the following:
> <?php $a = array('a', 'b', 'c', 'a'=>0, 'b'=>1, 'c'=>2); > sort($a); > print_r($a);
?>>
> This produces a bogus output:
> Array > ( > [0] => a > [1] => b > [2] => 0 > [3] => c > [4] => 1 > [5] => 2 > )
> Notice how 0 and c are switched incorrectly. Attached is a patch to > zend_operators.c that fixes it.
The current order simply makes no sense at all.ŽThe following though would: 0 a b c 1 2 // zero dirst, then strings then numbers a b c 0 1 2 // strings first, then numbers 0 1 2 a b c // numbers first, then strings and btw, john you forgot the patch, it is attached here. The one provided does the 2nd which means we only ensured 0 is treated un the same way other numbers are. Best regards, Marcus mailto:helly@php.net

Andi Gutmans

22 years ago
I talked to John about this yesterday. I'll take a closer look at this within the coming days to see if this makes sense and how it affects BC in general (if at all). At 09:45 PM 7/27/2004 +0200, Marcus Boerger wrote:

Andrey Hristov

22 years ago
Andi Gutmans wrote:
> I talked to John about this yesterday. I'll take a closer look at this > within the coming days to see if this makes sense and how it affects BC > in general (if at all). >
Can you take a look also at http://bugs.php.net/bug.php?id=29421 thanks, andrey

Marcus Börger

22 years ago
Hello Andrey, Friday, July 30, 2004, 11:30:18 AM, you wrote:
> Andi Gutmans wrote: >> I talked to John about this yesterday. I'll take a closer look at this >> within the coming days to see if this makes sense and how it affects BC >> in general (if at all). >> > Can you take a look also at http://bugs.php.net/bug.php?id=29421
We don't affect those. We simply correct the sorting to something understandable when it comes to edge cases with zero. We don't touch any other automatic typeconversion related behavior as mentioned in that but. php -r 'var_dump(array_search("baz",array("foo",TRUE ,"bar")));' The above returns int(1) because php -r 'var_dump((bool)"baz");' returns bool(true). Maybe we need a builtin search function that uses idetentity comparison? regards marcus

Andrey Hristov

22 years ago
Marcus Boerger wrote:
> Hello Andrey, > >> >>Can you take a look also at http://bugs.php.net/bug.php?id=29421 > > > We don't affect those. We simply correct the sorting to something > understandable when it comes to edge cases with zero. We don't touch > any other automatic typeconversion related behavior as mentioned in > that but. > > php -r 'var_dump(array_search("baz",array("foo",TRUE ,"bar")));' > > The above returns int(1) because > > php -r 'var_dump((bool)"baz");' > > returns bool(true). > > Maybe we need a builtin search function that uses idetentity comparison?
Ugh? What kind of comparison?
> regards > marcus >
thanks andrey

Andi Gutmans

22 years ago
How would you expect the sort function to behave on the following: array(true, false, 0, 1)? It is a different way of looking at the same problem and you are likely to hit this anytime you start comparing/sorting different data types. In general, I don't think it makes very much sense to fix the comparison operator for these special cases. PHP has always done this kind of type juggling and you would be changing a lot of its behavior. I suggest that when you do try and sort an array with different kind of data you define your own sorting callback function. Making special cases for each such case in the compare function will not only change behavior but it will slow down the already "too big for my taste" compare function. I know it's not the answer John was hoping for (I promised him to look into it more deeply when I'm back from OSCON) but after doing so and thinking about it, I think we'd be going in a direction which wouldn't be good. I have a feeling this might also not give you the expected behavior for something like array(0, "-1") but I haven't actually ran it. Andi At 09:45 PM 7/27/2004 +0200, Marcus Boerger wrote:

Sterling Hughes

22 years ago
i don't think sort() should be changed - this is how php works, for better or sometimes worse, changing it any other way would break BC, and it doesn't make much sense. -Sterling On Fri, 30 Jul 2004 15:39:19 -0700, Andi Gutmans <andi@zend.com> wrote:

John Coggeshall

22 years ago
I didn't realize that completely inconsistent behavior was no longer considered a bug, and this edge case is certainly inconsistent with the behavior of any other set of values sorted. Using the BC reasoning when something is broken (i.e. it's not a bug, its a "feature") makes even less sense. John On Tue, 2004-08-10 at 15:19, Sterling Hughes wrote:

Marcus Börger

22 years ago
Hello Sterling, Tuesday, August 10, 2004, 9:19:33 PM, you wrote:
> i don't think sort() should be changed - this is how php works, for > better or sometimes worse, changing it any other way would break BC, > and it doesn't make much sense.
Yep, meanwhile i think that if at all we could privide a 'nice' comparison function inside ext/standard. marcus

Andi Gutmans

22 years ago
At 09:32 PM 8/10/2004 +0200, Marcus Boerger wrote:
>Hello Sterling, > >Tuesday, August 10, 2004, 9:19:33 PM, you wrote: > > > i don't think sort() should be changed - this is how php works, for > > better or sometimes worse, changing it any other way would break BC, > > and it doesn't make much sense. > >Yep, meanwhile i think that if at all we could privide a 'nice' comparison >function inside ext/standard.
John, I think my explanation explains the problem and why I don't think it should be fixed. I agree with Sterling on this one. Andi

Derick Rethans

22 years ago
On Tue, 10 Aug 2004, Andi Gutmans wrote:
> I think my explanation explains the problem and why I don't think it should > be fixed. > I agree with Sterling on this one.
I concur too. Derick

Rasmus Lerdorf

22 years ago
On Tue, 10 Aug 2004, Sterling Hughes wrote:
> i don't think sort() should be changed - this is how php works, for > better or sometimes worse, changing it any other way would break BC, > and it doesn't make much sense.
I agree that we should keep the generic sort function the same. Might be nice to add some more sort flags though. It's somewhat annoying that the sort_string sort option doesn't use strcoll, for example, so you have to use a usort to get proper string sorting in most languages. -Rasmus

Sterling Hughes

22 years ago
no argument there, having a few more reasonable sort options/methods makes sense to me, so long as they offer new functionality (as opposed to fixing an "inconsistency") and don't affect the default behaviour... -sterling On Tue, 10 Aug 2004 12:59:09 -0700 (PDT), Rasmus Lerdorf <rasmus@php.net> wrote:

John Coggeshall

22 years ago
Which generic sort function? They all use the same sorting code as far as I can tell. John On Tue, 2004-08-10 at 15:59, Rasmus Lerdorf wrote: