From deefbe39c41f42e628361421a063ffe5def431df Mon Sep 17 00:00:00 2001 From: Tjerk Meesters Date: Thu, 4 Jun 2015 00:14:48 +0800 Subject: [PATCH 1/6] Add support for arrays of objects when used with array_column() --- ext/standard/array.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index a8e56c07cde03..1a0e3abe410da 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3090,11 +3090,16 @@ PHP_FUNCTION(array_column) array_init(return_value); ZEND_HASH_FOREACH_VAL(arr_hash, data) { ZVAL_DEREF(data); - if (Z_TYPE_P(data) != IS_ARRAY) { + if (Z_TYPE_P(data) == IS_OBJECT) { + if (zcolumn) { + /* properties are always string based */ + convert_to_string_ex(zcolumn); + } + } else if (Z_TYPE_P(data) != IS_ARRAY) { /* Skip elemens which are not sub-arrays */ continue; } - ht = Z_ARRVAL_P(data); + ht = HASH_OF(data); if (!zcolumn) { /* NULL column ID means use entire subarray as data */ @@ -3112,10 +3117,16 @@ PHP_FUNCTION(array_column) /* Failure will leave zkeyval alone which will land us on the final else block below * which is to append the value as next_index */ - if (zkey && (Z_TYPE_P(zkey) == IS_STRING)) { - zkeyval = zend_hash_find(ht, Z_STR_P(zkey)); - } else if (zkey && (Z_TYPE_P(zkey) == IS_LONG)) { - zkeyval = zend_hash_index_find(ht, Z_LVAL_P(zkey)); + if (zkey) { + if (Z_TYPE_P(data) == IS_OBJECT) { + /* properties are always string based */ + convert_to_string_ex(zkey); + } + if (Z_TYPE_P(zkey) == IS_STRING) { + zkeyval = zend_hash_find(ht, Z_STR_P(zkey)); + } else if (Z_TYPE_P(zkey) == IS_LONG) { + zkeyval = zend_hash_index_find(ht, Z_LVAL_P(zkey)); + } } Z_TRY_ADDREF_P(zcolval); From 995059c9edefca45717c0a20ddf1523635b2cafd Mon Sep 17 00:00:00 2001 From: Tjerk Meesters Date: Thu, 4 Jun 2015 23:07:53 +0800 Subject: [PATCH 2/6] Fixed issues with indirect zval types, added test case --- ext/standard/array.c | 2 +- .../array/array_column_variant_objects.phpt | 142 ++++++++++++++++++ 2 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 ext/standard/tests/array/array_column_variant_objects.phpt diff --git a/ext/standard/array.c b/ext/standard/array.c index 1a0e3abe410da..a65e61491bec0 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3123,7 +3123,7 @@ PHP_FUNCTION(array_column) convert_to_string_ex(zkey); } if (Z_TYPE_P(zkey) == IS_STRING) { - zkeyval = zend_hash_find(ht, Z_STR_P(zkey)); + zkeyval = zend_hash_find_ind(ht, Z_STR_P(zkey)); } else if (Z_TYPE_P(zkey) == IS_LONG) { zkeyval = zend_hash_index_find(ht, Z_LVAL_P(zkey)); } diff --git a/ext/standard/tests/array/array_column_variant_objects.phpt b/ext/standard/tests/array/array_column_variant_objects.phpt new file mode 100644 index 0000000000000..087020ac08dda --- /dev/null +++ b/ext/standard/tests/array/array_column_variant_objects.phpt @@ -0,0 +1,142 @@ +--TEST-- +Test array_column() function: testing with objects +--FILE-- +id = $id; + $this->first_name = $first_name; + $this->last_name = $last_name; + } +} + +function newUser($id, $first_name, $last_name) +{ + $o = new stdClass; + $o->{0} = $id; + $o->{1} = $first_name; + $o->{2} = $last_name; + + return $o; +} + +$records = array( + newUser(1, 'John', 'Doe'), + newUser(2, 'Sally', 'Smith'), + newUser(3, 'Jane', 'Jones'), + new User(1, 'John', 'Doe'), + new User(2, 'Sally', 'Smith'), + new User(3, 'Jane', 'Jones'), +); + +echo "*** Testing array_column() : object property fetching (numeric property names) ***\n"; + +echo "-- first_name column from recordset --\n"; +var_dump(array_column($records, 1)); + +echo "-- id column from recordset --\n"; +var_dump(array_column($records, 0)); + +echo "-- last_name column from recordset, keyed by value from id column --\n"; +var_dump(array_column($records, 2, 0)); + +echo "-- last_name column from recordset, keyed by value from first_name column --\n"; +var_dump(array_column($records, 2, 1)); + +echo "*** Testing array_column() : object property fetching (string property names) ***\n"; + +echo "-- first_name column from recordset --\n"; +var_dump(array_column($records, 'first_name')); + +echo "-- id column from recordset --\n"; +var_dump(array_column($records, 'id')); + +echo "-- last_name column from recordset, keyed by value from id column --\n"; +var_dump(array_column($records, 'last_name', 'id')); + +echo "-- last_name column from recordset, keyed by value from first_name column --\n"; +var_dump(array_column($records, 'last_name', 'first_name')); + +echo "Done\n"; +?> +--EXPECTF-- +*** Testing array_column() : object property fetching (numeric property names) *** +-- first_name column from recordset -- +array(3) { + [0]=> + string(4) "John" + [1]=> + string(5) "Sally" + [2]=> + string(4) "Jane" +} +-- id column from recordset -- +array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) +} +-- last_name column from recordset, keyed by value from id column -- +array(3) { + [1]=> + string(3) "Doe" + [2]=> + string(5) "Smith" + [3]=> + string(5) "Jones" +} +-- last_name column from recordset, keyed by value from first_name column -- +array(3) { + ["John"]=> + string(3) "Doe" + ["Sally"]=> + string(5) "Smith" + ["Jane"]=> + string(5) "Jones" +} +*** Testing array_column() : object property fetching (string property names) *** +-- first_name column from recordset -- +array(3) { + [0]=> + string(4) "John" + [1]=> + string(5) "Sally" + [2]=> + string(4) "Jane" +} +-- id column from recordset -- +array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) +} +-- last_name column from recordset, keyed by value from id column -- +array(3) { + [1]=> + string(3) "Doe" + [2]=> + string(5) "Smith" + [3]=> + string(5) "Jones" +} +-- last_name column from recordset, keyed by value from first_name column -- +array(3) { + ["John"]=> + string(3) "Doe" + ["Sally"]=> + string(5) "Smith" + ["Jane"]=> + string(5) "Jones" +} +Done From 8ec9a8096d45a0da43521ab58d34f53273ba7a27 Mon Sep 17 00:00:00 2001 From: Tjerk Meesters Date: Thu, 4 Jun 2015 23:40:45 +0800 Subject: [PATCH 3/6] Using zend_read_property() instead of zend_hash_find() --- ext/standard/array.c | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index a65e61491bec0..244884a3c1a1d 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3092,26 +3092,33 @@ PHP_FUNCTION(array_column) ZVAL_DEREF(data); if (Z_TYPE_P(data) == IS_OBJECT) { if (zcolumn) { + zval rv; /* properties are always string based */ convert_to_string_ex(zcolumn); + zcolval = zend_read_property(Z_OBJCE_P(data), data, Z_STRVAL_P(zcolumn), Z_STRLEN_P(zcolumn), 1, &rv); + if (Z_TYPE_P(zcolval) == IS_NULL) { + continue; + } + } + } else if (Z_TYPE_P(data) == IS_ARRAY) { + ht = Z_ARRVAL_P(data); + if (zcolumn) { + if (Z_TYPE_P(zcolumn) == IS_STRING && + (zcolval = zend_hash_find(ht, Z_STR_P(zcolumn))) == NULL) { + continue; + } else if (Z_TYPE_P(zcolumn) == IS_LONG && + (zcolval = zend_hash_index_find(ht, Z_LVAL_P(zcolumn))) == NULL) { + continue; + } } - } else if (Z_TYPE_P(data) != IS_ARRAY) { - /* Skip elemens which are not sub-arrays */ + } else { + /* not an array or object */ continue; } - ht = HASH_OF(data); if (!zcolumn) { /* NULL column ID means use entire subarray as data */ zcolval = data; - - /* Otherwise, skip if the value doesn't exist in our subarray */ - } else if ((Z_TYPE_P(zcolumn) == IS_STRING) && - ((zcolval = zend_hash_find(ht, Z_STR_P(zcolumn))) == NULL)) { - continue; - } else if ((Z_TYPE_P(zcolumn) == IS_LONG) && - ((zcolval = zend_hash_index_find(ht, Z_LVAL_P(zcolumn))) == NULL)) { - continue; } /* Failure will leave zkeyval alone which will land us on the final else block below @@ -3119,13 +3126,16 @@ PHP_FUNCTION(array_column) */ if (zkey) { if (Z_TYPE_P(data) == IS_OBJECT) { + zval rv; /* properties are always string based */ convert_to_string_ex(zkey); - } - if (Z_TYPE_P(zkey) == IS_STRING) { - zkeyval = zend_hash_find_ind(ht, Z_STR_P(zkey)); - } else if (Z_TYPE_P(zkey) == IS_LONG) { - zkeyval = zend_hash_index_find(ht, Z_LVAL_P(zkey)); + zkeyval = zend_read_property(Z_OBJCE_P(data), data, Z_STRVAL_P(zkey), Z_STRLEN_P(zkey), 1, &rv); + } else { + if (Z_TYPE_P(zkey) == IS_STRING) { + zkeyval = zend_hash_find_ind(ht, Z_STR_P(zkey)); + } else if (Z_TYPE_P(zkey) == IS_LONG) { + zkeyval = zend_hash_index_find(ht, Z_LVAL_P(zkey)); + } } } From 61d605c4fec86ee9ac2a1fd464111814714d138f Mon Sep 17 00:00:00 2001 From: Tjerk Meesters Date: Fri, 5 Jun 2015 00:41:30 +0800 Subject: [PATCH 4/6] Fixed issue with existing properties with null values --- ext/standard/array.c | 64 ++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 244884a3c1a1d..154ccccca71fe 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3068,6 +3068,29 @@ zend_bool array_column_param_helper(zval *param, } /* }}} */ +static inline zval *array_column_fetch_prop(zval *data, zval *name) +{ + zval *prop = NULL; + + if (Z_TYPE_P(data) == IS_OBJECT) { + zend_string *key = zval_get_string(name); + zval rv; + + if (!Z_OBJ_HANDLER_P(data, has_property) || Z_OBJ_HANDLER_P(data, has_property)(data, name, 2, NULL)) { + prop = zend_read_property(Z_OBJCE_P(data), data, key->val, key->len, 1, &rv); + } + zend_string_release(key); + } else if (Z_TYPE_P(data) == IS_ARRAY) { + if (Z_TYPE_P(name) == IS_STRING) { + prop = zend_hash_find(Z_ARRVAL_P(data), Z_STR_P(name)); + } else if (Z_TYPE_P(name) == IS_LONG) { + prop = zend_hash_index_find(Z_ARRVAL_P(data), Z_LVAL_P(name)); + } + } + + return prop; +} + /* {{{ proto array array_column(array input, mixed column_key[, mixed index_key]) Return the values from a single column in the input array, identified by the value_key and optionally indexed by the index_key */ @@ -3090,53 +3113,18 @@ PHP_FUNCTION(array_column) array_init(return_value); ZEND_HASH_FOREACH_VAL(arr_hash, data) { ZVAL_DEREF(data); - if (Z_TYPE_P(data) == IS_OBJECT) { - if (zcolumn) { - zval rv; - /* properties are always string based */ - convert_to_string_ex(zcolumn); - zcolval = zend_read_property(Z_OBJCE_P(data), data, Z_STRVAL_P(zcolumn), Z_STRLEN_P(zcolumn), 1, &rv); - if (Z_TYPE_P(zcolval) == IS_NULL) { - continue; - } - } - } else if (Z_TYPE_P(data) == IS_ARRAY) { - ht = Z_ARRVAL_P(data); - if (zcolumn) { - if (Z_TYPE_P(zcolumn) == IS_STRING && - (zcolval = zend_hash_find(ht, Z_STR_P(zcolumn))) == NULL) { - continue; - } else if (Z_TYPE_P(zcolumn) == IS_LONG && - (zcolval = zend_hash_index_find(ht, Z_LVAL_P(zcolumn))) == NULL) { - continue; - } - } - } else { - /* not an array or object */ - continue; - } if (!zcolumn) { - /* NULL column ID means use entire subarray as data */ zcolval = data; + } else if ((zcolval = array_column_fetch_prop(data, zcolumn)) == NULL) { + continue; } /* Failure will leave zkeyval alone which will land us on the final else block below * which is to append the value as next_index */ if (zkey) { - if (Z_TYPE_P(data) == IS_OBJECT) { - zval rv; - /* properties are always string based */ - convert_to_string_ex(zkey); - zkeyval = zend_read_property(Z_OBJCE_P(data), data, Z_STRVAL_P(zkey), Z_STRLEN_P(zkey), 1, &rv); - } else { - if (Z_TYPE_P(zkey) == IS_STRING) { - zkeyval = zend_hash_find_ind(ht, Z_STR_P(zkey)); - } else if (Z_TYPE_P(zkey) == IS_LONG) { - zkeyval = zend_hash_index_find(ht, Z_LVAL_P(zkey)); - } - } + zkeyval = array_column_fetch_prop(data, zkey); } Z_TRY_ADDREF_P(zcolval); From a429a43f069c47b795f56c6432cd82164710b478 Mon Sep 17 00:00:00 2001 From: Tjerk Meesters Date: Fri, 5 Jun 2015 01:14:00 +0800 Subject: [PATCH 5/6] Fixed __get()/__isset() behaviour --- ext/standard/array.c | 13 ++++++------- .../array/array_column_variant_objects.phpt | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 154ccccca71fe..8406d6031bedc 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3068,16 +3068,15 @@ zend_bool array_column_param_helper(zval *param, } /* }}} */ -static inline zval *array_column_fetch_prop(zval *data, zval *name) +static inline zval *array_column_fetch_prop(zval *data, zval *name, zval *rv) { zval *prop = NULL; if (Z_TYPE_P(data) == IS_OBJECT) { zend_string *key = zval_get_string(name); - zval rv; - if (!Z_OBJ_HANDLER_P(data, has_property) || Z_OBJ_HANDLER_P(data, has_property)(data, name, 2, NULL)) { - prop = zend_read_property(Z_OBJCE_P(data), data, key->val, key->len, 1, &rv); + if (!Z_OBJ_HANDLER_P(data, has_property) || Z_OBJ_HANDLER_P(data, has_property)(data, name, 1, NULL)) { + prop = zend_read_property(Z_OBJCE_P(data), data, key->val, key->len, 1, rv); } zend_string_release(key); } else if (Z_TYPE_P(data) == IS_ARRAY) { @@ -3098,7 +3097,7 @@ PHP_FUNCTION(array_column) { zval *zcolumn = NULL, *zkey = NULL, *data; HashTable *arr_hash; - zval *zcolval = NULL, *zkeyval = NULL; + zval *zcolval = NULL, *zkeyval = NULL, rvc, rvk; HashTable *ht; if (zend_parse_parameters(ZEND_NUM_ARGS(), "hz!|z!", &arr_hash, &zcolumn, &zkey) == FAILURE) { @@ -3116,7 +3115,7 @@ PHP_FUNCTION(array_column) if (!zcolumn) { zcolval = data; - } else if ((zcolval = array_column_fetch_prop(data, zcolumn)) == NULL) { + } else if ((zcolval = array_column_fetch_prop(data, zcolumn, &rvc)) == NULL) { continue; } @@ -3124,7 +3123,7 @@ PHP_FUNCTION(array_column) * which is to append the value as next_index */ if (zkey) { - zkeyval = array_column_fetch_prop(data, zkey); + zkeyval = array_column_fetch_prop(data, zkey, &rvk); } Z_TRY_ADDREF_P(zcolval); diff --git a/ext/standard/tests/array/array_column_variant_objects.phpt b/ext/standard/tests/array/array_column_variant_objects.phpt index 087020ac08dda..1acf2a62191ee 100644 --- a/ext/standard/tests/array/array_column_variant_objects.phpt +++ b/ext/standard/tests/array/array_column_variant_objects.phpt @@ -25,6 +25,19 @@ function newUser($id, $first_name, $last_name) return $o; } +class Something +{ + public function __isset($name) + { + return $name == 'id'; + } + + public function __get($name) + { + return "something"; + } +} + $records = array( newUser(1, 'John', 'Doe'), newUser(2, 'Sally', 'Smith'), @@ -32,6 +45,7 @@ $records = array( new User(1, 'John', 'Doe'), new User(2, 'Sally', 'Smith'), new User(3, 'Jane', 'Jones'), + new Something, ); echo "*** Testing array_column() : object property fetching (numeric property names) ***\n"; @@ -113,13 +127,15 @@ array(3) { string(4) "Jane" } -- id column from recordset -- -array(3) { +array(4) { [0]=> int(1) [1]=> int(2) [2]=> int(3) + [3]=> + string(9) "something" } -- last_name column from recordset, keyed by value from id column -- array(3) { From 2ab9b6260a277479145f5bafdfbd93401541cd80 Mon Sep 17 00:00:00 2001 From: Tjerk Meesters Date: Fri, 5 Jun 2015 02:23:20 +0800 Subject: [PATCH 6/6] Fixed memory leaks --- ext/standard/array.c | 7 ++++++- .../array/array_column_variant_objects.phpt | 19 +++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 8406d6031bedc..16636f972fa72 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3098,7 +3098,6 @@ PHP_FUNCTION(array_column) zval *zcolumn = NULL, *zkey = NULL, *data; HashTable *arr_hash; zval *zcolval = NULL, *zkeyval = NULL, rvc, rvk; - HashTable *ht; if (zend_parse_parameters(ZEND_NUM_ARGS(), "hz!|z!", &arr_hash, &zcolumn, &zkey) == FAILURE) { return; @@ -3138,6 +3137,12 @@ PHP_FUNCTION(array_column) } else { add_next_index_zval(return_value, zcolval); } + if (zcolval == &rvc) { + zval_ptr_dtor(&rvc); + } + if (zkeyval == &rvk) { + zval_ptr_dtor(&rvk); + } } ZEND_HASH_FOREACH_END(); } /* }}} */ diff --git a/ext/standard/tests/array/array_column_variant_objects.phpt b/ext/standard/tests/array/array_column_variant_objects.phpt index 1acf2a62191ee..80e1839736f70 100644 --- a/ext/standard/tests/array/array_column_variant_objects.phpt +++ b/ext/standard/tests/array/array_column_variant_objects.phpt @@ -29,12 +29,12 @@ class Something { public function __isset($name) { - return $name == 'id'; + return $name == 'first_name'; } public function __get($name) { - return "something"; + return new User(4, 'Jack', 'Sparrow'); } } @@ -118,24 +118,31 @@ array(3) { } *** Testing array_column() : object property fetching (string property names) *** -- first_name column from recordset -- -array(3) { +array(4) { [0]=> string(4) "John" [1]=> string(5) "Sally" [2]=> string(4) "Jane" + [3]=> + object(User)#8 (3) { + ["id"]=> + int(4) + ["first_name"]=> + string(4) "Jack" + ["last_name"]=> + string(7) "Sparrow" + } } -- id column from recordset -- -array(4) { +array(3) { [0]=> int(1) [1]=> int(2) [2]=> int(3) - [3]=> - string(9) "something" } -- last_name column from recordset, keyed by value from id column -- array(3) {