Codeigniter: error insert_batch using PHP7

[iam-adty]

got this error when executing insert_batch using PHP7 but work without error when using PHP5.6

Severity: Warning
Message: array_keys() expects parameter 1 to be array, boolean given
Filename: database/DB_query_builder.php
Line Number: 1549

it seem that current($key) always returning FALSE on line 1549 (using PHP7)
$keys = array_keys($this->_object_to_array(current($key)));

for a quick fix, i added
reset($key)
before line 1549.

has anyone experiencing this or it's just me?

[narfbg]

How exactly are you using it?

[iam-adty]

thanks for asking. thinkin again about how exactly i am using it, i just realize the mistake on my code.

this is how i use it:
`public function insert_language($general_data = [], $language_data = [])
{
$insert_id = 0;
$status = NULL;

$parent = [];
$child = [];
foreach ($language_data as $key => $value)
{
    if(empty($parent))
    {
        $parent = array_merge($general_data, $value, ['language' => $key]);
    }
    else
    {
        $child[] = array_merge($general_data, $value, ['language' => $key]);
    }
}

$this->db->trans_start();

$insert_parent = $this->db->insert($this->table, $parent);
$parent_id = $this->db->insert_id();

array_walk($child, function(&$item, $key, $parent_id){
    $item['parent_id'] = $parent_id;
}, $parent_id);

$insert_child = $this->db->insert_batch($this->table, $child);

$this->db->trans_complete();
$status = $this->db->trans_status();

return [
    'status' => $status,
    'id' => $parent_id,
    'child_count' => $insert_child,
    'error' => $this->db->error()
];

}`

the error occurred because the use array_walk on the array before executing insert_batch. the array_walk cause the index of the array out of bound. if i add
reset($child)
before insert_batch, it works normal.

my point here is the different behavior of PHP5.6 and PHP7. on PHP5.6, my code works without resetting the index of the array. but on PHP7 i have to reset the index of the array.

what do you think about this? i think there should be a code to prevent this case.

[narfbg]

I don't see a behavioral change between PHP 5.6 and 7.0: https://3v4l.org/aV0PR

[iam-adty]

OK, no behavioral change. It has to be my machine then. But, the test return a bool(false) value on 5.6 and 7.0. It should return an array value, isn't? The function _object_to_array() don't accept value other than object, so it will return the original value. If this function got a bool(false) value, which is returned by current($key) if the index is out of bound, a warning message will appear because array_keys() only accept array. Then the generated SQL won't be perfect. It's missing field names.

Error Number: 1136
Column count doesn't match value count at row 1
INSERT INTO prudentialkh_content () VALUES ('','[]','en',77,99,'draft','2016-09-09 13:35:04','','EVENT')
Filename: core/MY_Model.php
Line Number: 804

[narfbg]

... I'm not sure I understand what you're trying to say.

[iam-adty]

I'm sorry for my bad language or explanation. Try this code on your machine with PHP 5.6 and 7.0,

$data = [
[ 'name' => 'john'],
['name' => 'doe']
];
array_walk($data, function(&$item){
$item['email'] = $item['name'] . '@email.com';
});
$this->db->insert_batch('user', $data);

don't bother with error 'no database salected'. i intentionally left database name blank.

I got this when using PHP 5.6

and for PHP 7.0

FYI. I use vagrant ubuntu/xenial64 with PHP from ppa:ondrej/php
hope it help.

[narfbg]

Please don't use screenshots.

Yes, I see that a broken input causes a broken result - that's obvious. What I don't understand is what you claim is the problem and why.

Are you still talking about a portability issue between PHP 5.6 and 7.0?
Or are you talking about a different issue?

[iam-adty]

Sorry for my late response. Actually i don't know what issue is. But i don't think it is a portability issue between PHP 5.6 and 7.0 because the on your test it returns the same value.

I think the problem here is the array handling in CI. You see, if i use array_walk() before $this->db->insert_batch() it would fail because the array index is out of bound. At first, I think this is a portability issue between PHP 5.6 and 7.0. But when I look and test my code again and also read the warning message generated by CI, it isn't it. The root of the error mentioned by the warning message is in the CI system. On the other hand, the documentation didn't mention to reset my array before going into $this->db->insert_batch(). It's a bit confusing for me.

So, if you get what i'm saying here. I'd like to suggest to add a reset/checking code in CI system for array value provided by the user before going into loops or array operation.

[narfbg]

We accept arrays in thousands of places and if we reset() here, we should do it everywhere ...

Please come back again when you can more clearly define the problem.

[iam-adty]

OK. Thanks @narfbg for your response. Case closed.

[narfbg]

This is not a support tickets system, we don't handle issues based on individuals' problems. There's likely a bug here, it's just unknown what exactly.

[tianhe1986]

@narfbg @iam-adty
I tried this, with passing variable in the same way as in CI_DB_query_builder . https://3v4l.org/kgkll
It seems that internal pointer was reset when passing array to other function with PHP 5.6, but remained with PHP 7.0.

[vlakoff]

See this snippet. There is a change starting from PHP 5.3.14, 5.4.4 and 7.0.0. Note PHP 7.1.0 changes back to the previous behavior.

About the issue here, though in one small snippet PHP 5.6 and 7.0 show the same behavior, it should be considered as undetermined.

The proper way to get the first element is to use reset() instead of current(). The function returns the element as well. It resets the array pointer, but usually it affects the array only inside the function scope (unless it is passed by reference, or returned).

And if you really don't want to affect the array pointer:

function head($array) {
    return reset($array);
}

[vlakoff]

You can also do this:

// Initialize if array is possibly empty
$first_key = null;
$first_value = null;

foreach ($array as $key => $value) {
    $first_key = $key;
    $first_value = $value;
    break;
}

Alternative, but be cautious of line order:

// Line order is important!
$first_value = reset($array);
$first_key = key($array);

[narfbg]

@vlakoff So, the cleanest patch possible would be this?

diff --git a/system/database/DB_query_builder.php b/system/database/DB_query_builder.php
index 3f1a802..ab19d97 100644
--- a/system/database/DB_query_builder.php
+++ b/system/database/DB_query_builder.php
@@ -1553,7 +1553,7 @@ abstract class CI_DB_query_builder extends CI_DB_driver {

                is_bool($escape) OR $escape = $this->_protect_identifiers;

-               $keys = array_keys($this->_object_to_array(current($key)));
+               $keys = array_keys($this->_object_to_array(reset($key)));
                sort($keys);

                foreach ($key as $row)

If so, it looks very reasonable to me regardless of what causes the issue.

But the problem so far for me has been that everybody is discussing the CI internals, and nobody actually giving an accurate description of what the issue is. Is it just developers passing arrays with modified pointers, or does CI move that pointer somewhere?

[vlakoff]

Yes, this is the fix to apply.

Some quick demo of the issue:

$users = [
    ['name' => 'foo'],
    ['name' => 'bar'],
];

while ($user = next($users)) { // or array_walk(), etc.
    // ...
}

// now array pointer is after the last element

$qb->insert_batch('users_table', $users); // boom