Bug #76587 [Fbk->Csd]: array_column gives incorrect result if $indexKey selects a float

From: Date: Sun, 18 Nov 2018 00:23:22 +0000
Subject: Bug #76587 [Fbk->Csd]: array_column gives incorrect result if $indexKey selects a float
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-218007@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=76587&edit=1 ID: 76587 Updated by: cmb@php.net Reported by: rowan dot collins at gmail dot com Summary: array_column gives incorrect result if $indexKey selects a float -Status: Feedback +Status: Closed Type: Bug Package: Arrays related PHP Version: 7.2.7 -Assigned To: +Assigned To: cmb Block user comment: N Private report: N New Comment: Since this has been fixed by laruence's commit, I'm closing this ticket. Previous Comments: ------------------------------------------------------------------------ [2018-07-24 03:39:24] laruence@php.net should be fixed in : https://github.com/php/php-src/commit/38d97557da2614f5fe1d0fa64f33dfb2cf6cbaad ------------------------------------------------------------------------ [2018-07-06 14:41:20] rowan dot collins at gmail dot com Hm, good spot on the expected behaviour; I'd based it on the polyfill, but cast-to-int (which is what HHVM's implementation does) is indeed the more consistent behaviour; the cast-to-string could therefore be considered a bug in the polyfill. My actual use case involved a database result set where I'd used FLOOR(some_calculation), which resulted in values like float(1.0), so either behaviour would have been fine. I worked around the problem by using CAST(some_calculation AS INT) instead. The problem with the current implementation is that it has the effect of *completely ignoring* the $indexKey value, which is extremely surprising, and I can't see why that would be desirable (if you wanted the keys generated sequentially, you would omit the $indexKey parameter). As for overwriting values, that's inevitable because a) you could have any number of identical index values anyway, and b) there will always be cases like int(0) and string("0") which are not distinguished. ------------------------------------------------------------------------ [2018-07-06 14:35:12] cmb@php.net Related To: Bug #76589 ------------------------------------------------------------------------ [2018-07-06 13:57:21] danack@php.net The issue seems to be the code in array_column appears to not cater for floats at all and to instead use add_next_index_zval to add the next value, if the key was going to be a float. if (Z_TYPE_P(zkeyval) == IS_STRING) { zend_symtable_update(Z_ARRVAL_P(return_value), Z_STR_P(zkeyval), zcolval); } else if (Z_TYPE_P(zkeyval) == IS_LONG) { add_index_zval(return_value, Z_LVAL_P(zkeyval), zcolval); } else if (Z_TYPE_P(zkeyval) == IS_OBJECT) { zend_string *tmp_key; zend_string *key = zval_get_tmp_string(zkeyval, &tmp_key); zend_symtable_update(Z_ARRVAL_P(return_value), key, zcolval); zend_tmp_string_release(tmp_key); } else { add_next_index_zval(return_value, zcolval); } Possibly related: https://bugs.php.net/bug.php?id=68553 Just to note, even if array_column worked as it possibly should - for your case it still won't give you the expected output as floats get cast to ints for array keys: ``` $foo = [ 2.5 => 'whatever' ]; var_dump($foo); //output is: array(1) { [2] => string(8) "whatever" } ``` As such - it's possible to see why that behaviour was chosen. Using add_next_index_zval avoids overwriting columns in some cases......but, ewwww - not all: ``` $a = [ [ 'foo' => 0.5 ], [ 'foo' => 0 ], [ 'foo' => '0' ], ]; var_dump(array_column($a, 'foo', 'foo')); // output is array(1) { [0]=> string(1) "0" } ``` ------------------------------------------------------------------------ [2018-07-06 13:56:31] cmb@php.net I can confirm the described behavior: <https://3v4l.org/Yvaup>. However, I'm not sure that qualifies as bug, since the docs[1] state: | This value may be the integer key of the column, or it may be | the string key name. Anyhow, if we would change the current behavior (BC break!), we 'd had to adapt this code[2] (cast FLOAT to INT), or perhaps even treat the $column_key parameter in the same way and adapt array_column_fetch_prop()[3]. [1] <http://php.net/manual/en/function.array-column.php> [2] <https://github.com/php/php-src/blob/af341213f73650b28b74b374501d84060eb604ab/ext/standard/array.c#L4177-L4182> [3] <https://github.com/php/php-src/blob/af341213f73650b28b74b374501d84060eb604ab/ext/standard/array.c#L4111-L4115> ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=76587 -- Edit this bug report at https://bugs.php.net/bug.php?id=76587&edit=1

« previous php.bugs (#218007) next »