Broken bulkBind in mysqli driver

[sgregor]

Hi,

with this commit 507466e an Execute method has been added to drivers/adodb-mysqli.inc.php. This broke bulkBind for Execute using the mysqli driver.

I tried to figure out why it has been added but could not find any answers...

[dregad]

@sgregor I'm not saying there is no problem with bulkBind(), but your conclusions are incorrect - the execute() method was not introduced by 507466e (which is just a revert commit), but by a4b5110 which added support for bound variable statements in MySQL (see #655).

[sgregor]

@dregad Thank you for your answer. I did not spot that commit in git.

Here is a small script that works with 5.21 but does not with 5.22:

require_once 'adodb.inc.php';

/*
* Simple connection
*/
$driver = 'mysqli';
$db     = adoNewConnection($driver);
/*
* Now connect to the database
*/
$host = 'localhost';
$user = 'user';
$password = 'password';
$database = 'employees';
$db->connect($host, $user, $password, $database);

$db->debug = true;
$db->Execute("DROP TABLE IF EXISTS `adodb_test`;");
$db->Execute("CREATE TABLE `adodb_test` (
    `column1` int(11) NOT NULL,
    `column2` int(11) NOT NULL
  );");

$db->bulkBind = true;

$db->Execute("INSERT INTO `adodb_test` (column1, column2) VALUES (?, ?)", [[1,2], [3,4], [5,6]]);

[mnewnham]

This has created 2 problems:

1. The execute() method uses the length of the input array to determine the bind parameter type for each column. With bulkbind, the length of the input array represents the number of rows to import, not the number of columns in the array
2. The _query() method needs to iterate over the input array to push each record into the db

[mnewnham]

So there is a problem in determining the data types in the bulkbind if we want to take advantage of the true binding, We look at the data types of each element of the array to determine the bind types:

$ar = array('A','B','C',1,2,3);

So the bind types would be sssiii

But if the second line of the bulk bind was like this (because column 4 is actually a string not an integer):

$ar = array('A','B','C','D',2,3);

but we used the derived version from the first row, then the insert would fail, because the types should be: ssssii .

We have 2 choices I think:

1. Iterate over every row in the array and generate a type list for that specific row(I'm assuming that this doesn't give mysql a problem). This would be no faster than pushing each row of the bulk array into execute manually.
2. Provide a mechanism to manually push the type list into the method. This would be the most efficient, but would be custom for mysql

[mnewnham]

I have pushed a fix that attempts to resolve this issue. Please test if possible. In addition, it adds a performance feature that allows you to pre-define the column type string, instead of forcing the driver to iterate over all column of all rows of the bulk insert. So instead of:

$db->bulkBind = true;

Which is still mandatory with bulk binding, use a string that represents the mysqli bind param type

$db->bulkBind = 'ssssii';

The value binding operation will use this format instead of iterating over all rows of the bulkbind, attempting to determine the data type. This will save a little cpu time. If used this way, the bulkBind flag will be immediately reset after the query execution.

[sgregor]

@mnewnham I tested your fix (2f4dc1c) and it works fine for my use case.

Thank you for your quick reaction!

[dregad]

Iterate over every row in the array and generate a type list for that specific row

I'm wondering if this is really necessary, depending on the size of the input array this could mean a lot of iterations...

IMO it would make sense and be a reasonable requirement to report on the caller, the burden of ensuring that the bind data consistently has the right type for each item across the array's rows. In this case we could derive the bind types from row[0] and just let the bulk operation fail if there is an incorrect type in another row.

[mnewnham]

Per my original example. our function guesses the type list from what it sees as data in the first row, so I would say a guess of an integer value that could be a string in the second row would be immediately suspect

[nickendle]

I was experiencing a problem with my prepared statements and bulk bind as well -- 2f4dc1c seems to have resolved this, but now calling $db->affected_rows() is only returning 1 instead of the number of inserted rows...

[mnewnham]

This is a problem with the mysqli driver. Unlike say, oracle where a bulk insert of 60 records is a single transaction of 60 records, the mysql driver executes a single row 60 times, the affected rows is not cumulative. this article on stackoverflow explains it,