cycle / database

Database Abstraction Layer, Schema Introspection, Schema Generation, Query Builders
MIT License
53 stars 22 forks source link

Reuse of PDOStatement when query cache is enabled results in SQLite "21 bad parameter or other API misuse" for file connections #131

Open whmcs-ben opened 10 months ago

whmcs-ben commented 10 months ago

No duplicates 🥲.

Database

SQLite

What happened?

When query caching is enabled, if a constraint violation is encountered during an insert statement, a subsequent attempt to perform the same insert query will reuse the PDOStatement object stored in the cache, and its reuse will result in SQLite producing the following error:

PHP Fatal error: Uncaught PDOException: SQLSTATE[HY000]: General error: 21 bad parameter or other API misuse in vendor/cycle/database/src/Driver/Driver.php:444

This situation occurs under two additional conditions:

  1. The SQLite database is a file.
  2. The SQLite file is opened, not created.

Reproduction

I've attached a test script. The output will consist of o, record inserted, or x constraint violated. Execution of this script under different conditions follows.

<?php
declare(strict_types=1);

use Cycle\Database\Config\DatabaseConfig;
use Cycle\Database\Config\SQLite\FileConnectionConfig;
use Cycle\Database\Config\SQLite\MemoryConnectionConfig;
use Cycle\Database\Config\SQLiteDriverConfig;
use Cycle\Database\DatabaseManager;
use Cycle\Database\Exception\StatementException\ConstrainException;
use Cycle\ORM\EntityManager;
use Cycle\ORM\Factory;
use Cycle\ORM\Mapper\StdMapper;
use Cycle\ORM\ORM;
use Cycle\ORM\Schema;
use Cycle\ORM\SchemaInterface;

require __DIR__ . '/vendor/autoload.php';

$driver = new SQLiteDriverConfig(
    connection: new FileConnectionConfig(
        database: __DIR__ . '/blah.db',
    ),
    reconnect: true,
    queryCache: true,
);
//$driver = new SQLiteDriverConfig(
//  connection: new MemoryConnectionConfig(),
//  reconnect: true,
//  queryCache: true,
//);

$databaseName = 'default';
$dm = new DatabaseManager(new DatabaseConfig([
    'default' => $databaseName,
    'databases' => [
        $databaseName => ['connection' => 'sqlite']
    ],
    'connections' => ['sqlite' => $driver]
]));

$tableName = 'testing';

$table = $dm->database('default')
    ->table($tableName)
    ->getSchema();
$table->setPrimaryKeys(['a', 'b']);
$table->string('a')->nullable(false);
$table->string('b')->nullable(false);
$table->save();
unset($table);

$orm = new ORM(
    new Factory($dm),
    new Schema([
        $tableName => [
            SchemaInterface::MAPPER => StdMapper::class,
            SchemaInterface::DATABASE => 'default',
            SchemaInterface::TABLE => $tableName,
            SchemaInterface::PRIMARY_KEY => ['a', 'b'],
            SchemaInterface::COLUMNS => [
                'a' => 'a',
                'b' => 'b',
            ],
        ]
    ]),
);

$things = ['123', '123', '456'];
foreach ($things as $value) {
    $em = new EntityManager($orm);
    $record = $orm->make($tableName);
    $record->a = 'static';
    $record->b = $value;
    $em->persist($record);
    print '_';
    try {
        $em->run();
        print chr(8) . 'o';
    } catch (ConstrainException) {
        // 23000: UNIQUE constraint failed
        print chr(8) . 'x';
    }
}
print "\n";

Fresh database file, queryCache: false

$ rm blah.db
$ php testorm.php
oxo

Fresh database file, queryCache: true

$ rm blah.db
$ php testorm.php
oxo

Existing database file, queryCache: false

$ php testorm.php 
xxx

Existing database file, queryCache: true

$ php testorm.php
x_PHP Fatal error:  Uncaught PDOException: SQLSTATE[HY000]: General error: 21 bad parameter or other API misuse in /home/whmcs/sync/soylent/vendor/cycle/database/src/Driver/Driver.php:444
Stack trace:
#0 vendor/cycle/database/src/Driver/Driver.php(444): PDOStatement->execute()
#1 vendor/cycle/database/src/Driver/Driver.php(270): Cycle\Database\Driver\Driver->statement()
#2 vendor/cycle/database/src/Query/InsertQuery.php(125): Cycle\Database\Driver\Driver->execute()
#3 vendor/cycle/orm/src/Command/Database/Insert.php(85): Cycle\Database\Query\InsertQuery->run()
#4 vendor/cycle/orm/src/Transaction/Runner.php(61): Cycle\ORM\Command\Database\Insert->execute()
#5 vendor/cycle/orm/src/Transaction/UnitOfWork.php(150): Cycle\ORM\Transaction\Runner->run()
#6 vendor/cycle/orm/src/Transaction/UnitOfWork.php(327): Cycle\ORM\Transaction\UnitOfWork->runCommand()
#7 vendor/cycle/orm/src/Transaction/UnitOfWork.php(375): Cycle\ORM\Transaction\UnitOfWork->resolveSelfWithEmbedded()
#8 vendor/cycle/orm/src/Transaction/UnitOfWork.php(219): Cycle\ORM\Transaction\UnitOfWork->resolveRelations()
#9 vendor/cycle/orm/src/Transaction/UnitOfWork.php(96): Cycle\ORM\Transaction\UnitOfWork->walkPool()
#10 vendor/cycle/orm/src/EntityManager.php(47): Cycle\ORM\Transaction\UnitOfWork->run()
#11 testorm.php(77): Cycle\ORM\EntityManager->run()
#12 {main}

Expectation

Regardless of whether the SQLite database file is created within the same process, a constraint violation in a prepared INSERT statement should not result in the same INSERT query later failing; whether it would encounter another constraint violation or not.

Version

PHP 8.1.23 (cli) (built: Sep  2 2023 06:59:15) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.23, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.23, Copyright (c), by Zend Technologies

cycle/database                   2.5.2   DBAL, schema introspection, migration and pagination
cycle/orm                        v2.3.4  PHP DataMapper ORM and Data Modelling Engine

$ php -i | grep -i sqlite
/etc/php/8.1/cli/conf.d/20-pdo_sqlite.ini,
/etc/php/8.1/cli/conf.d/20-sqlite3.ini,
PDO drivers => sqlite
pdo_sqlite
PDO Driver for SQLite 3.x => enabled
SQLite Library => 3.37.2
sqlite3
SQLite3 support => enabled
SQLite Library => 3.37.2
sqlite3.defensive => On => On
sqlite3.extension_dir => no value => no value
whmcs-ben commented 10 months ago

I am happy to submit this against the ORM repository if some investigation points to it being the culprit. As my discovery led me to the query cache and discussions on the net regarding reuse of PDOStatement objects, I felt like starting at the driver level might be prudent.

Thank you for your time and work!