php / php-src

The PHP Interpreter
https://www.php.net
Other
38.15k stars 7.74k forks source link

MySQLi statement execute ArgumentCountError ("number of parameters") #13809

Closed iaibai closed 6 months ago

iaibai commented 7 months ago

Description

We've been having an issue where mysqli_stmt::execute is intermittently throwing an ArgumentCountError despite that we have successfully bound the correct number of parameters using bind_param. The error happens intermittently - one particular statement was failing about 50% of the time, a couple of others have failed once in hundreds or thousands of calls.

The following code:

<?php
$stmt = $dbConn->prepare("INSERT INTO `hot_availability`.`unit_availability`
    (`property_unittype_id`,`property_id`,`avail_date`,`num_avail`,
    `creation_time`,`username`,`active`)
    VALUES(?,?,?,?,UNIX_TIMESTAMP(),?,'Y')");
$stmt->bind_param('iisis',$propUnitId,$propertyId,$availDate,$numAvail,$username);
$stmt->execute(); // error here

Resulted in this output:

[2024-03-26 02:25:07] production.ERROR: The number of variables must match the number of parameters in the prepared statement {"exception":"[object] (ArgumentCountError(code: 0): The number of variables must match the number of parameters in the prepared statement at /var/app/current/class/channel/ChannelAvailability.class.php:1430)
[stacktrace]
#0 [internal function]: mysqli_stmt->bind_param()
#1 /var/app/current/class/channel/ChannelAvailability.class.php(1430): mysqli_stmt->execute()
#2 /var/app/current/class/channel/ChannelRateSync.class.php(415): ChannelAvailability::setRatesAndAvailability()
#3 /var/app/current/app/Channel/RateSyncWorker.php(35): ChannelRateSync::runRateSync()
#4 /var/app/current/app/Worker.php(89): App\\Channel\\RateSyncWorker->work()
#5 /var/app/current/auto/channel/rate_sync_worker.php(9): App\\Worker->daemon()
#6 {main}

But I expected the statement to execute correctly and return true. We also have "mysqli_report(MYSQLI_REPORT_OFF);", so I wouldn't have expected execute() to throw an Error.

The prepared statements that have errored have been part of our codebase for years, and thi only started when we moved from PHP 7.4 to PHP 8.2.

We managed to stop the one that was erroring 50% of the time simply by remove a space character that was at the start of the query, which made me think it was maybe some kind of caching? Removing a space character that was at the end of the query however did not help.

Although we use Laravel and its' query builder, a lot of our application is legacy code that uses MySQLi, in object-oriented style. We have not had any issues with the Laravel query builder queries - just MySQLi prepared statements.

I have not been able to find any report of a similar error, aside from people who have genuinely not set the correct number of parameters.

PHP Version

PHP 8.2.15

Operating System

Amazon Linux 2023

iaibai commented 7 months ago

This issue may be related: https://github.com/matomo-org/matomo/issues/19413

SakiTakamachi commented 7 months ago

I tried imagining the missing information and supplementing it with local, but it doesn't reproduce even after 1000 loops.

Can you provide code that allows me to reproduce the problem by simply copying and running the code, including the process of creating a table and preparing dummy values ​​for variables? Of course, the authentication information can be dummy.

iaibai commented 7 months ago

Thanks, it is proving difficult to replicate as it's very intermittent but I'll give it a go.

It has been happening on multiple different prepared statements and I've yet to see any sort of pattern. Since I made this issue it has happened only once - to a totally different query than the one I posted.

iaibai commented 7 months ago

I'm afraid we've not been able to reliably recreate this so far, but it is still happening on multiple different statements.

We're attempting to test if there's a possibility it happens when the server is under load as there was a bit of a burst of them last night, but so far our load testing hasn't produced any results.

The stack trace suggests the error is coming from a bind_param call inside execute, but we never provide any arguments to the execute() function (because we use bind_param() instead), so I'm not sure why it would ever be doing that.

SakiTakamachi commented 7 months ago

From what I've seen of the implementation, it looks like your code won't cause any errors...

Is the actual production code also written with SQL literals, like in your example? Also, is the number of variables passed to bind_param fixed like in the code example?

iaibai commented 7 months ago

Most of the queries that have failed at some point do have SQL literals in them, but this one for example doesn't:

$stmt = $dbConn->prepare("DELETE
    FROM ta_review_queue
    WHERE book_id=?");
if ( $stmt ) {
    $stmt->bind_param('s',$bookId);
    foreach ( $bookIds as $bookId ) {
        if ( !$stmt->execute() ) { // <-- threw ArgumentCountError on this line
            error_log($stmt->error." in ".__FILE__." on ".__LINE__);
        }
    }
    $stmt->close();
}

Normally the number of variables passed to bind_param is fixed. I think all the ones that have errored have had a fixed number of parameters

Currently we're getting the issue fairly consistently (about a third of the time) on this query (this is actual production code).

// Failed fax count this week
$failFaxCount = 0;
$ts = strtotime("today - 7 days");
$stringTs = date('Y-m-d H:i:s', $ts);

$dbConn = DbConnFactory::getDbConn();
$stmt = $dbConn->prepare(" SELECT COUNT(bc.bc_id)
    FROM bookings b
    INNER JOIN booking_comms bc ON bc.b_id=b.b_id
    LEFT JOIN booking_fax_records bfr ON bfr.bc_id=bc.bc_id
    WHERE bc.comm_type='fax'
    AND b.creation_date > ?
    AND (bc.fax_transaction_id IS NULL OR bfr.status_code != 0)
    ORDER BY bc.bc_id ASC ");
if (! $stmt) {
    throw new Exception("Prepare Failed : " . $dbConn->error);
}
$stmt->bind_param('s', $stringTs);
if (! $stmt->execute()) {
    throw new Exception("Execute Failed : " . $stmt->error);
}
$stmt->bind_result($failFaxCount);
$stmt->fetch();
$stmt->close();

Oddly, if we do not execute the previous query on the page then it doesn't fail. The previous query is (currently with hard-coded values and even those fail):

        try {
            $qry = "SELECT fb_id
FROM feedback
WHERE 1
AND type = ?
AND date_sent > ?
AND processed = ?
ORDER BY date_sent DESC";

            $stmt = $dbConn->prepare($qry);

            $type = "UF";
            $sentAfter = "2024-03-21 15:57:09";
            $processed = "N";

            $stmt->bind_param("sss", $type, $sentAfter, $processed);
            $stmt->execute();
            $stmt->bind_result($fbId);
            $stmt->fetch();
            $stmt->close();
        } catch (Exception $e) {
            Log::exceptionError($e);
            return 0;
        }

Our method for creating $dbConn is:


public static function getDbConn( $connectionType = self::READONLY_DB )
{
    // check if we already have made a connection of this type
    // no point making more than one of the same
    if (isset(self::$dbConns[$connectionType]) && self::$dbConns[$connectionType] instanceof mysqli && self::$dbConns[$connectionType]->ping())
    {
        return self::$dbConns[$connectionType];
    }

    if ( isset(self::$dbConns[$connectionType]) )
    {
        self::$reconnections++;
        if ( self::$reconnections>0 )
        {
            //error_log(self::$reconnections." reconnections.");
        }
    }

    // otherwise get the params for the connection
    if ( $args = self::dbConnParams($connectionType) )
    {
        mysqli_report(MYSQLI_REPORT_OFF);

        // connect
        $dbConn = @new mysqli(
            $args['host'],
            $args['user'],
            $args['password'],
            $args['database'],
            $args['port'] ?? ini_get("mysqli.default_port"),
            $args['socket'] ?? ini_get("mysqli.default_socket")
        );
        if ( $error = mysqli_connect_error() )
        {
            error_log("connect error: $error in :: {$_SERVER['HTTP_HOST']}{$_SERVER['REQUEST_URI']}");
            return false;
        }
        $dbConn->set_charset('utf8mb4');
        self::$dbConns[$connectionType] = $dbConn;
        return $dbConn;
    }

    error_log("Cannot find db params for $connectionType in ".__FILE__." on ".__LINE__);
    return false;

}
SakiTakamachi commented 7 months ago

I can't reproduce it at all. Honestly, the only idea I can think of was to use NTS for multi-threading...

nielsdos commented 6 months ago

If it's flaky it could also be a memory corruption bug that eventually leads to this. If it repros on CLI, I would try running with Valgrind with the environment variable USE_ZEND_ALLOC=0.

iaibai commented 6 months ago

Just to keep you updated, we now believe this is caused by the newrelic plugin. I've created an issue for them to look into it: https://github.com/newrelic/newrelic-php-agent/issues/875