diff --git a/NEWS b/NEWS index b2e8d22465bf..647003f0d508 100644 --- a/NEWS +++ b/NEWS @@ -76,6 +76,10 @@ PHP NEWS . Fixed bug GH-23444 (ODBC_ATTR_ASSUME_UTF8 corrupts Unicode data outside Windows). (Calvin Buckley, Lazizbek Ergashev) +- PDO_PGSQL: + . Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist. + (KentarouTakeda) + - PDO Sqlite: . Fixed bug GH-20214 (PDO::FETCH_DEFAULT unexpected behavior with PDOStatement::setFetchMode). (SakiTakamachi) diff --git a/ext/pdo_pgsql/config.m4 b/ext/pdo_pgsql/config.m4 index 1137a911c811..eb002e3f38ac 100644 --- a/ext/pdo_pgsql/config.m4 +++ b/ext/pdo_pgsql/config.m4 @@ -25,6 +25,12 @@ if test "$PHP_PDO_PGSQL" != "no"; then or later).])],, [$PGSQL_LIBS]) + PHP_CHECK_LIBRARY([pq], [PQclosePortal], + [AC_DEFINE([HAVE_PQCLOSEPORTAL], [1], + [Define to 1 if libpq has the 'PQclosePortal' function (PostgreSQL 17 + or later).])],, + [$PGSQL_LIBS]) + PHP_CHECK_PDO_INCLUDES PHP_NEW_EXTENSION([pdo_pgsql], diff --git a/ext/pdo_pgsql/config.w32 b/ext/pdo_pgsql/config.w32 index aec5db508d07..fcc60140d31f 100644 --- a/ext/pdo_pgsql/config.w32 +++ b/ext/pdo_pgsql/config.w32 @@ -9,6 +9,10 @@ if (PHP_PDO_PGSQL != "no") { AC_DEFINE('HAVE_PDO_PGSQL', 1, "Define to 1 if the PHP extension 'pdo_pgsql' is available."); + if (GREP_HEADER("libpq-fe.h", "PQclosePortal", PHP_PDO_PGSQL + "\\include;" + PHP_PHP_BUILD + "\\include\\pgsql;" + PHP_PHP_BUILD + "\\include\\libpq;")) { + AC_DEFINE('HAVE_PQCLOSEPORTAL', 1, "Define to 1 if libpq has the 'PQclosePortal' function (PostgreSQL 17 or later)."); + } + ADD_EXTENSION_DEP('pdo_pgsql', 'pdo'); ADD_MAKEFILE_FRAGMENT(); } else { diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index 8f3dd5237b5a..8a33f5e91b72 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -56,7 +56,47 @@ #define FLOAT8LABEL "float8" #define FLOAT8OID 701 +#ifndef HAVE_PQCLOSEPORTAL +static bool pdo_pgsql_try_cmd(const char *cmd, const char *ok_sqlstate, pdo_pgsql_db_handle *H) +{ + bool result = false; + char *q = NULL; + PGresult *res = NULL; + + PGTransactionStatusType status = PQtransactionStatus(H->server); + + switch (status) { + case PQTRANS_ACTIVE: + case PQTRANS_INERROR: + break; + case PQTRANS_INTRANS: /* failure must not abort the caller's transaction */ + /* PQexec does not run the statements following a failed one */ + spprintf(&q, 0, "SAVEPOINT pdo_pgsql_savepoint; %s; RELEASE SAVEPOINT pdo_pgsql_savepoint;", cmd); + res = PQexec(H->server, q); + + if (PQresultStatus(res) != PGRES_COMMAND_OK) { + PQclear(PQexec(H->server, "ROLLBACK TO SAVEPOINT pdo_pgsql_savepoint; RELEASE SAVEPOINT pdo_pgsql_savepoint")); + } + + break; + default: + res = PQexec(H->server, cmd); + } + + if (PQresultStatus(res) == PGRES_COMMAND_OK) { + result = true; + } else if (res) { + const char *sqlstate = pdo_pgsql_sqlstate(res); + result = sqlstate && !strcmp(sqlstate, ok_sqlstate); + } + + if (q) efree(q); + if (res) PQclear(res); + + return result; +} +#endif static int pgsql_stmt_dtor(pdo_stmt_t *stmt) { @@ -114,15 +154,16 @@ static int pgsql_stmt_dtor(pdo_stmt_t *stmt) } if (S->cursor_name) { - if (server_obj_usable) { + if (S->is_cursor_declared && server_obj_usable) { pdo_pgsql_db_handle *H = S->H; - char *q = NULL; - PGresult *res; - +#ifndef HAVE_PQCLOSEPORTAL + char *q; spprintf(&q, 0, "CLOSE %s", S->cursor_name); - res = PQexec(H->server, q); + pdo_pgsql_try_cmd(q, "34000", H); /* 34000: invalid_cursor_name */ efree(q); - if (res) PQclear(res); +#else + PQclear(PQclosePortal(H->server, S->cursor_name)); +#endif } efree(S->cursor_name); S->cursor_name = NULL; @@ -156,10 +197,25 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt) if (S->cursor_name) { char *q = NULL; - if (S->is_prepared) { + if (S->is_cursor_declared) { +#ifndef HAVE_PQCLOSEPORTAL spprintf(&q, 0, "CLOSE %s", S->cursor_name); - PQclear(PQexec(H->server, q)); + + /* 34000: invalid_cursor_name */ + if (pdo_pgsql_try_cmd(q, "34000", H)) { + S->is_cursor_declared = false; + } + efree(q); +#else + PGresult *res = PQclosePortal(H->server, S->cursor_name); + + if (PQresultStatus(res) == PGRES_COMMAND_OK) { + S->is_cursor_declared = false; + } + + PQclear(res); +#endif } spprintf(&q, 0, "DECLARE %s SCROLL CURSOR WITH HOLD FOR %s", S->cursor_name, ZSTR_VAL(stmt->active_query_string)); @@ -175,7 +231,7 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt) PQclear(S->result); /* the cursor was declared correctly */ - S->is_prepared = 1; + S->is_cursor_declared = true; /* fetch to be able to get the number of tuples later, but don't advance the cursor pointer */ spprintf(&q, 0, "FETCH FORWARD 0 FROM %s", S->cursor_name); diff --git a/ext/pdo_pgsql/php_pdo_pgsql_int.h b/ext/pdo_pgsql/php_pdo_pgsql_int.h index fc9f1664cc3d..c4ea431da21b 100644 --- a/ext/pdo_pgsql/php_pdo_pgsql_int.h +++ b/ext/pdo_pgsql/php_pdo_pgsql_int.h @@ -68,6 +68,7 @@ typedef struct { Oid *param_types; int current_row; bool is_prepared; + bool is_cursor_declared; } pdo_pgsql_stmt; typedef struct { diff --git a/ext/pdo_pgsql/tests/cursor_scroll_close_failed_before_redeclare.phpt b/ext/pdo_pgsql/tests/cursor_scroll_close_failed_before_redeclare.phpt new file mode 100644 index 000000000000..1877b0be1390 --- /dev/null +++ b/ext/pdo_pgsql/tests/cursor_scroll_close_failed_before_redeclare.phpt @@ -0,0 +1,44 @@ +--TEST-- +PDO PgSQL PDO::CURSOR_SCROLL keeps track of a held cursor when the CLOSE before a re-declare fails +--EXTENSIONS-- +pdo_pgsql +--SKIPIF-- + +--FILE-- +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" diff --git a/ext/pdo_pgsql/tests/cursor_scroll_discard_all.phpt b/ext/pdo_pgsql/tests/cursor_scroll_discard_all.phpt new file mode 100644 index 000000000000..216766ddb9cf --- /dev/null +++ b/ext/pdo_pgsql/tests/cursor_scroll_discard_all.phpt @@ -0,0 +1,37 @@ +--TEST-- +PDO PgSQL PDO::CURSOR_SCROLL cursor destroyed by DISCARD ALL does not break the transaction +--EXTENSIONS-- +pdo_pgsql +--SKIPIF-- + +--FILE-- +setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + +$stmt = $db->prepare('SELECT 1', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]); +$stmt->execute(); + +/* a connection pooler issues this when handing the connection back */ +$db->exec('DISCARD ALL'); + +$db->beginTransaction(); + +unset($stmt); + +echo $db->query('SELECT 2')->fetchColumn(), PHP_EOL; + +$db->rollBack(); + +echo 'Done', PHP_EOL; + +?> +--EXPECT-- +2 +Done diff --git a/ext/pdo_pgsql/tests/cursor_scroll_failed_redeclare.phpt b/ext/pdo_pgsql/tests/cursor_scroll_failed_redeclare.phpt new file mode 100644 index 000000000000..a836294e03cb --- /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-- + +--FILE-- +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']); + +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 diff --git a/ext/pdo_pgsql/tests/cursor_scroll_reexecute_after_rollback.phpt b/ext/pdo_pgsql/tests/cursor_scroll_reexecute_after_rollback.phpt new file mode 100644 index 000000000000..e434d74fdda3 --- /dev/null +++ b/ext/pdo_pgsql/tests/cursor_scroll_reexecute_after_rollback.phpt @@ -0,0 +1,40 @@ +--TEST-- +PDO PgSQL PDO::CURSOR_SCROLL re-execute after a rollback destroyed the cursor +--EXTENSIONS-- +pdo_pgsql +--SKIPIF-- + +--FILE-- +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(); + +$stmt->execute(); +echo $stmt->fetchColumn(), PHP_EOL; + +echo $db->query('SELECT 2')->fetchColumn(), PHP_EOL; + +$db->rollBack(); + +echo 'Done', PHP_EOL; + +?> +--EXPECT-- +1 +2 +Done diff --git a/ext/pdo_pgsql/tests/cursor_scroll_rollback_destroyed.phpt b/ext/pdo_pgsql/tests/cursor_scroll_rollback_destroyed.phpt new file mode 100644 index 000000000000..8dc6f7620048 --- /dev/null +++ b/ext/pdo_pgsql/tests/cursor_scroll_rollback_destroyed.phpt @@ -0,0 +1,34 @@ +--TEST-- +PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE for a cursor a rollback already destroyed +--EXTENSIONS-- +pdo_pgsql +--SKIPIF-- + +--FILE-- +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 diff --git a/ext/pdo_pgsql/tests/cursor_scroll_without_declare.phpt b/ext/pdo_pgsql/tests/cursor_scroll_without_declare.phpt new file mode 100644 index 000000000000..437ea341b0cf --- /dev/null +++ b/ext/pdo_pgsql/tests/cursor_scroll_without_declare.phpt @@ -0,0 +1,29 @@ +--TEST-- +PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE for a cursor it never declared +--EXTENSIONS-- +pdo_pgsql +--SKIPIF-- + +--FILE-- +setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + +$db->beginTransaction(); + +$stmt = $db->prepare('SELECT 1', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]); +unset($stmt); + +$db->exec('SELECT 2'); + +echo 'Done'; + +?> +--EXPECT-- +Done