Revert "Pdo\Pgsql: Fix getColumnMeta() for GH-15287 Pdo\Pgsql::setAttribute(PDO::ATTR_PREFETCH, 0)"

This reverts commit 8982351.

Author: devnexen

NEWS

@@ -7,8 +7,4 @@ PHP                                                                        NEWS
     with a given skeleton, locale, collapse type and identity fallback.
     (BogdanUngureanu)
 
-- PDO_PGSQL:
-  . Fixed Pdo\Pgsql::getColumnMeta() when Pdo\Pgsql::setAttribute(PDO::ATTR_PREFETCH, 0).
-    (outtersg)
-
 <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>

ext/pdo_pgsql/pgsql_driver.c

@@ -36,7 +36,6 @@
 #include "pgsql_driver_arginfo.h"
 
 static bool pgsql_handle_in_transaction(pdo_dbh_t *dbh);
-void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode);
 
 static char * _pdo_pgsql_trim_message(const char *message, int persistent)
 {
@@ -110,37 +109,6 @@ int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const char *
 }
 /* }}} */
 
-static zend_always_inline void pgsql_finish_running_stmt(pdo_pgsql_db_handle *H)
-{
-	if (H->running_stmt) {
-		pgsql_stmt_finish(H->running_stmt, 0);
-	}
-}
-
-static zend_always_inline void pgsql_discard_running_stmt(pdo_pgsql_db_handle *H)
-{
-	if (H->running_stmt) {
-		pgsql_stmt_finish(H->running_stmt, FIN_DISCARD);
-	}
-
-	PGresult *pgsql_result;
-	bool first = true;
-	while ((pgsql_result = PQgetResult(H->server))) {
-		/* We should not arrive here, where libpq has a result to deliver without us
-		 * having registered a running statement:
-		 * every result discarding should go through the unified pgsql_stmt_finish,
-		 * but maybe there still is an internal query that we omitted to adapt.
-		 * So instead of asserting let's just emit an informational notice,
-		 * and consume anyway (results consumption is handle-wise, so we have no formal
-		 * need for the statement). */
-		if (first) {
-			php_error_docref(NULL, E_NOTICE, "Internal error: unable to link a libpq result to consume, to its origin statement");
-			first = false;
-		}
-		PQclear(pgsql_result);
-	}
-}
-
 static void _pdo_pgsql_notice(void *context, const char *message) /* {{{ */
 {
 	pdo_dbh_t * dbh = (pdo_dbh_t *)context;
@@ -290,10 +258,6 @@ static void pgsql_handle_closer(pdo_dbh_t *dbh) /* {{{ */
 			PQfinish(H->server);
 			H->server = NULL;
 		}
-		if (H->cached_table_name) {
-			efree(H->cached_table_name);
-			H->cached_table_name = NULL;
-		}
 		if (H->einfo.errmsg) {
 			pefree(H->einfo.errmsg, dbh->is_persistent);
 			H->einfo.errmsg = NULL;
@@ -387,7 +351,6 @@ static zend_long pgsql_handle_doer(pdo_dbh_t *dbh, const zend_string *sql)
 
 	bool in_trans = pgsql_handle_in_transaction(dbh);
 
-	pgsql_finish_running_stmt(H);
 	if (!(res = PQexec(H->server, ZSTR_VAL(sql)))) {
 		/* fatal error */
 		pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL);
@@ -463,7 +426,6 @@ static zend_string *pdo_pgsql_last_insert_id(pdo_dbh_t *dbh, const zend_string *
 	PGresult *res;
 	ExecStatusType status;
 
-	pgsql_finish_running_stmt(H);
 	if (name == NULL) {
 		res = PQexec(H->server, "SELECT LASTVAL()");
 	} else {
@@ -627,7 +589,6 @@ static bool pdo_pgsql_transaction_cmd(const char *cmd, pdo_dbh_t *dbh)
 	PGresult *res;
 	bool ret = true;
 
-	pgsql_finish_running_stmt(H);
 	res = PQexec(H->server, cmd);
 
 	if (PQresultStatus(res) != PGRES_COMMAND_OK) {
@@ -735,8 +696,9 @@ void pgsqlCopyFromArray_internal(INTERNAL_FUNCTION_PARAMETERS)
 	/* Obtain db Handle */
 	H = (pdo_pgsql_db_handle *)dbh->driver_data;
 
-	pgsql_discard_running_stmt(H);
-
+	while ((pgsql_result = PQgetResult(H->server))) {
+		PQclear(pgsql_result);
+	}
 	pgsql_result = PQexec(H->server, query);
 
 	efree(query);
@@ -858,8 +820,9 @@ void pgsqlCopyFromFile_internal(INTERNAL_FUNCTION_PARAMETERS)
 
 	H = (pdo_pgsql_db_handle *)dbh->driver_data;
 
-	pgsql_discard_running_stmt(H);
-
+	while ((pgsql_result = PQgetResult(H->server))) {
+		PQclear(pgsql_result);
+	}
 	pgsql_result = PQexec(H->server, query);
 
 	efree(query);
@@ -953,7 +916,9 @@ void pgsqlCopyToFile_internal(INTERNAL_FUNCTION_PARAMETERS)
 		RETURN_FALSE;
 	}
 
-	pgsql_discard_running_stmt(H);
+	while ((pgsql_result = PQgetResult(H->server))) {
+		PQclear(pgsql_result);
+	}
 
 	/* using pre-9.0 syntax as PDO_pgsql is 7.4+ compatible */
 	if (pg_fields) {
@@ -1042,7 +1007,9 @@ void pgsqlCopyToArray_internal(INTERNAL_FUNCTION_PARAMETERS)
 
 	H = (pdo_pgsql_db_handle *)dbh->driver_data;
 
-	pgsql_discard_running_stmt(H);
+	while ((pgsql_result = PQgetResult(H->server))) {
+		PQclear(pgsql_result);
+	}
 
 	/* using pre-9.0 syntax as PDO_pgsql is 7.4+ compatible */
 	if (pg_fields) {
@@ -1494,7 +1461,6 @@ static int pdo_pgsql_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* {{{
 
 	H->attached = 1;
 	H->pgoid = -1;
-	H->cached_table_oid = InvalidOid;
 
 	dbh->methods = &pgsql_methods;
 	dbh->alloc_own_columns = 1;

ext/pdo_pgsql/pgsql_statement.c

@@ -56,14 +56,14 @@
 #define FLOAT8LABEL "float8"
 #define FLOAT8OID 701
 
+#define FIN_DISCARD 0x1
+#define FIN_CLOSE   0x2
+#define FIN_ABORT   0x4
 
 
-void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
-{
-	if (!S) {
-		return;
-	}
 
+static void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
+{
 	pdo_pgsql_db_handle *H = S->H;
 
 	if (S->is_running_unbuffered && S->result && (fin_mode & FIN_ABORT)) {
@@ -113,10 +113,9 @@ void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
 		}
 
 		S->is_prepared = false;
-	}
-
-	if (H->running_stmt == S && (fin_mode & (FIN_CLOSE|FIN_ABORT))) {
-		H->running_stmt = NULL;
+		if (H->running_stmt == S) {
+			H->running_stmt = NULL;
+		}
 	}
 }
 
@@ -189,8 +188,9 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt)
 	 * and returns a PGRES_FATAL_ERROR when PQgetResult gets called for stmt 2 if DEALLOCATE
 	 * was called for stmt 1 inbetween
 	 * (maybe it will change with pipeline mode in libpq 14?) */
-	if (H->running_stmt && H->running_stmt->is_unbuffered) {
+	if (S->is_unbuffered && H->running_stmt) {
 		pgsql_stmt_finish(H->running_stmt, FIN_CLOSE);
+		H->running_stmt = NULL;
 	}
 	/* ensure that we free any previous unfetched results */
 	pgsql_stmt_finish(S, 0);
@@ -702,29 +702,12 @@ static int pgsql_stmt_get_col(pdo_stmt_t *stmt, int colno, zval *result, enum pd
 	return 1;
 }
 
-static zend_always_inline char * pdo_pgsql_translate_oid_to_table(Oid oid, pdo_pgsql_db_handle *H)
+static zend_always_inline char * pdo_pgsql_translate_oid_to_table(Oid oid, PGconn *conn)
 {
-	PGconn *conn = H->server;
 	char *table_name = NULL;
 	PGresult *tmp_res;
 	char *querystr = NULL;
 
-	if (oid == H->cached_table_oid) {
-		return H->cached_table_name;
-	}
-
-	if (H->running_stmt && H->running_stmt->is_unbuffered) {
-		/* in single-row mode, libpq forbids passing a new query
-		 * while we're still flushing the current one's result */
-		return NULL;
-	}
-
-	if (H->cached_table_name) {
-		efree(H->cached_table_name);
-		H->cached_table_name = NULL;
-		H->cached_table_oid = InvalidOid;
-	}
-
 	spprintf(&querystr, 0, "SELECT RELNAME FROM PG_CLASS WHERE OID=%d", oid);
 
 	if ((tmp_res = PQexec(conn, querystr)) == NULL || PQresultStatus(tmp_res) != PGRES_TUPLES_OK) {
@@ -741,8 +724,6 @@ static zend_always_inline char * pdo_pgsql_translate_oid_to_table(Oid oid, pdo_p
 		return 0;
 	}
 
-	H->cached_table_oid = oid;
-	H->cached_table_name = estrdup(table_name);
 	table_name = estrdup(table_name);
 
 	PQclear(tmp_res);
@@ -771,9 +752,10 @@ static int pgsql_stmt_get_column_meta(pdo_stmt_t *stmt, zend_long colno, zval *r
 
 	table_oid = PQftable(S->result, colno);
 	add_assoc_long(return_value, "pgsql:table_oid", table_oid);
-	table_name = pdo_pgsql_translate_oid_to_table(table_oid, S->H);
+	table_name = pdo_pgsql_translate_oid_to_table(table_oid, S->H->server);
 	if (table_name) {
-		add_assoc_string(return_value, "table", S->H->cached_table_name);
+		add_assoc_string(return_value, "table", table_name);
+		efree(table_name);
 	}
 
 	switch (S->cols[colno].pgsql_type) {
@@ -812,10 +794,6 @@ static int pgsql_stmt_get_column_meta(pdo_stmt_t *stmt, zend_long colno, zval *r
 			break;
 		default:
 			/* Fetch metadata from Postgres system catalogue */
-			if (S->H->running_stmt && S->H->running_stmt->is_unbuffered) {
-				/* libpq forbids calling a query while we're still reading the preceding one's */
-				break;
-			}
 			spprintf(&q, 0, "SELECT TYPNAME FROM PG_TYPE WHERE OID=%u", S->cols[colno].pgsql_type);
 			res = PQexec(S->H->server, q);
 			efree(q);

ext/pdo_pgsql/php_pdo_pgsql_int.h

@@ -43,8 +43,6 @@ typedef struct {
 	unsigned 	_reserved:31;
 	pdo_pgsql_error_info	einfo;
 	Oid 		pgoid;
-	Oid cached_table_oid;
-	char *cached_table_name;
 	unsigned int	stmt_counter;
 	bool		emulate_prepares;
 	bool		disable_prepares;
@@ -92,12 +90,6 @@ extern int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const
 
 extern const struct pdo_stmt_methods pgsql_stmt_methods;
 
-#define FIN_DISCARD 0x1
-#define FIN_CLOSE   0x2
-#define FIN_ABORT   0x4
-
-extern void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode);
-
 #define pdo_pgsql_sqlstate(r) PQresultErrorField(r, PG_DIAG_SQLSTATE)
 
 enum {

ext/pdo_pgsql/tests/gh15287.phpt

@@ -132,17 +132,6 @@ $res = []; while (($re = $stmt->fetch())) $res[] = $re; display($res);
 $stmt->execute([ 0 ]);
 $res = []; for ($i = -1; ++$i < 2;) $res[] = $stmt->fetch(); display($res);
 display($pdo->query("select * from t2")->fetchAll());
-
-// Metadata calls the server for some operations (notably table oid-to-name conversion).
-// This will break libpq (that forbids a second PQexec before we consumed the first one).
-// Instead of either letting libpq return an error, or blindly forbid this call, we expect
-// being transparently provided at least attributes which do not require a server roundtrip.
-// And good news: column name is one of those "local" attributes.
-echo "=== meta ===\n";
-$stmt = $pdo->query("select * from t limit 2");
-echo "Starting with column " . $stmt->getColumnMeta(0)['name'] . ":\n";
-display($stmt->fetchAll());
-
 ?>
 --EXPECTF--
 === non regression ===
@@ -192,7 +181,3 @@ multiple calls to the same prepared statement, some interrupted before having re
 0
 1
 678	ok
-=== meta ===
-Starting with column n:
-0	original
-1	non original