Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
whmcs-ben opened this issue Sep 25, 2023 · 1 comment
Assignees
Labels
status:to be verified Needs to be reproduced and validated. type:bug Bug

Comments

@whmcs-ben
Copy link

whmcs-ben commented Sep 25, 2023

No duplicates 🥲.

  • I have searched for a similar issue in our bug tracker and didn't find any solutions.

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 whmcs-ben added status:to be verified Needs to be reproduced and validated. type:bug Bug labels Sep 25, 2023
@whmcs-ben
Copy link
Author

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!

@roxblnfk roxblnfk added this to Cycle Oct 6, 2023
@roxblnfk roxblnfk moved this to Todo in Cycle Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:to be verified Needs to be reproduced and validated. type:bug Bug
Projects
Status: Todo
Development

No branches or pull requests

3 participants