Skip to content

ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist - #23490

Open
KentarouTakeda wants to merge 1 commit into
php:PHP-8.4from
KentarouTakeda:fix-pdo-pgsql-cursor-dtor
Open

ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist#23490
KentarouTakeda wants to merge 1 commit into
php:PHP-8.4from
KentarouTakeda:fix-pdo-pgsql-cursor-dtor

Conversation

@KentarouTakeda

@KentarouTakeda KentarouTakeda commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

The destructor of a statement created with [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL] tries to close the cursor even when it does not exist. This fixes that.

The attempted close causes an error on the database server, but the destructor discards its result, so the error cannot be observed by the user. Apart from polluting the server's log, this is mostly harmless, but when it happens inside a transaction, it causes a strange situation where subsequent statements fail for a reason that cannot be observed.

is_prepared was overloaded to also mean "cursor declared" and was never reset, so the cursor state now has its own flag.

@KentarouTakeda KentarouTakeda changed the title Fixed PDO::CURSOR_SCROLL statements closing a cursor that was never declared ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that was never declared Aug 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--- /dev/null
+++ b/ext/pdo_pgsql/tests/cursor_scroll_failed_redeclare.phpt
@@ -0,0 +1,37 @@
+--TEST--
+PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE after a failed re-declare
+--EXTENSIONS--
+pdo_pgsql
+--SKIPIF--
+<?php
+require __DIR__ . '/config.inc';
+require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+
+require_once __DIR__ . "/config.inc";
+
+$db = Pdo::connect($config['ENV']['PDOTEST_DSN']);
+
+$stmt = $db->prepare('SELECT CAST(:v AS int)', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
+$stmt->execute([':v' => '1']);
+
+try {
+    $stmt->execute([':v' => 'not an int']);
+} catch (PDOException $e) {
+    echo $e::class, ': ', $e->getCode(), PHP_EOL;
+}
+
+$db->beginTransaction();
+unset($stmt);
+
+$db->exec('SELECT 2');
+
+echo 'Done', PHP_EOL;
+
+?>
+--EXPECT--
+PDOException: 22P02
+Done

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, your test failed as expected. I've changed the approach. is_prepared and is_cursor_declared now track the two states separately.

@KentarouTakeda
KentarouTakeda force-pushed the fix-pdo-pgsql-cursor-dtor branch from b5a49d1 to 0366dee Compare August 30, 2026 11:46
@KentarouTakeda KentarouTakeda changed the title ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that was never declared ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist. Aug 30, 2026
@KentarouTakeda
KentarouTakeda changed the base branch from master to PHP-8.4 August 30, 2026 11:47
@KentarouTakeda
KentarouTakeda force-pushed the fix-pdo-pgsql-cursor-dtor branch from 0366dee to fa968a0 Compare August 30, 2026 11:53
@KentarouTakeda KentarouTakeda changed the title ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist. ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist Aug 30, 2026
@KentarouTakeda

Copy link
Copy Markdown
Contributor Author

I had the wrong target branch. This bug is not new, so it needs to go to PHP-8.4.

@devnexen

Copy link
Copy Markdown
Member

Some more tests :)

--TEST--
PDO PgSQL PDO::CURSOR_SCROLL keeps track of a held cursor when the CLOSE before a re-declare fails
--EXTENSIONS--
pdo_pgsql
--SKIPIF--
<?php
require __DIR__ . '/config.inc';
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
PDOTest::skip();
?>
--FILE--
<?php

require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$stmt = $db->prepare('SELECT CAST(:v AS int)', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
$stmt->execute([':v' => '1']);

$db->beginTransaction();

try {
    $db->exec('SELECT 1 / 0');
} catch (PDOException $e) {
    echo $e::class, ': ', $e->getCode(), PHP_EOL;
}

try {
    $stmt->execute([':v' => '2']);
} catch (PDOException $e) {
    echo $e::class, ': ', $e->getCode(), PHP_EOL;
}

$db->rollBack();
unset($stmt);

var_dump($db->query("SELECT count(*) FROM pg_cursors WHERE name LIKE 'pdo\_crsr\_%'")->fetchColumn());

?>
--EXPECT--
PDOException: 22012
PDOException: 25P02
string(1) "0"
--TEST--
PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE for a cursor a rollback already destroyed
--EXTENSIONS--
pdo_pgsql
--SKIPIF--
<?php
require __DIR__ . '/config.inc';
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
PDOTest::skip();
?>
--FILE--
<?php

require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$db->beginTransaction();

$stmt = $db->prepare('SELECT 1', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
$stmt->execute();

$db->rollBack();

$db->beginTransaction();
unset($stmt);

$db->exec('SELECT 2');

echo 'Done', PHP_EOL;

?>
--EXPECT--
Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants