Yii2: Support iterable in `QueryBuilder::batchInsert`

[SamMousa]

What steps will reproduce the problem?

Pass an iterable to QueryBuilder::batchInsert().

What is the expected result?

That it works for both empty and non empty iterables.

What do you get instead?

Empty iterables create a bad query; the snippet below will never trigger for iterables since they are objects. Same goes for things like ArrayObject, they behave close to an object but they are never empty according to empty().

if (empty($rows)) { return ''; }

Additional info

This can be resolved by not checking if $rows is empty (one cannot check if an iterable is empty).
Instead we should just iterate over the rows and at the end check if $values is empty.
The overhead of this approach would be that the table schema gets initialized regardless of whether rows are empty or not.

I'm up for making a PR, just need to know if you agree with the tradeoff between usability and (theoretical) performance.

| Q | A
| ---------------- | ---
| Yii version | latest
| PHP version | N/A
| Operating system | N/A

[samdark]

Can't we check if it's an array and only then check for empty()?

[SamMousa]

Yes we can, so then you get 2 checks, 1 before 1 after?

[samdark]

No. I mean modifying a check the following way:

if (is_array($rows) && empty($rows)) {
    return '';
}

[SamMousa]

Yes but if it is not an array it can still be an empty iterable; so in that case we would need to check if $values are empty after the iteration.

[samdark]

Ah. Yes. That part won't change.

[SamMousa]

So you prefer to have 2 checks then so that we have the optimization of not creating the schema for empty arrays?

(In my opinion it is a minor / irrelevant optimization, how many people will be calling batchInsert repeatedly with empty datasets?)

[samdark]

Yes, I prefer two checks. I don't like de-optimizing our code.

[rugabarbo]

Strange issue.

@SamMousa said on Aug 7th, 2017:

we should just iterate over the rows and at the end check if $values is empty

But this has already been implemented earlier on Feb 24th, 2017 in this commit 6db6a2d.

In any case, I added an additional test, which shows that now everything works correctly (see PR #16990). Empty ArrayObject doesn't create a bad query.

@samdark I think this issue can be closed...