Promote invalid field to ValueError in pgsql

The same error condition is a ValueError in mysqli, be consistent.

Additionally, do not display the argument name for these errors.
As the signatures are overloaded, the argument name may not match
the meaning at all.

4 files changed, 124 insertions, 49 deletions

diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c
index 779c1f444b..b2b08f7fc1 100644
--- a/ext/pgsql/pgsql.c
+++ b/ext/pgsql/pgsql.c
@@ -1560,8 +1560,8 @@ PHP_FUNCTION(pg_field_table)
 	}
 
 	if (fnum >= PQnfields(pg_result->result)) {
-		php_error_docref(NULL, E_WARNING, "Bad field offset specified");
-		RETURN_FALSE;
+		zend_argument_value_error(2, "must be less than the number of fields for this result set");
+		RETURN_THROWS();
 	}
 
 	oid = PQftable(pg_result->result, (int)fnum);
@@ -1650,8 +1650,8 @@ static void php_pgsql_get_field_info(INTERNAL_FUNCTION_PARAMETERS, int entry_typ
 	pgsql_result = pg_result->result;
 
 	if (field >= PQnfields(pgsql_result)) {
-		php_error_docref(NULL, E_WARNING, "Bad field offset specified");
-		RETURN_FALSE;
+		zend_argument_value_error(2, "must be less than the number of fields for this result set");
+		RETURN_THROWS();
 	}
 
 	switch (entry_type) {
@@ -1728,6 +1728,29 @@ PHP_FUNCTION(pg_field_num)
 }
 /* }}} */
 
+static zend_long field_arg_to_offset(
+		PGresult *result, zend_string *field_name, zend_long field_offset, int arg_num) {
+	if (field_name) {
+		field_offset = PQfnumber(result, ZSTR_VAL(field_name));
+		if (field_offset < 0) {
+			/* Avoid displaying the argument name, as the signature is overloaded and the name
+			 * might not line up. */
+			zend_value_error("Argument #%d must be a field name from this result set", arg_num);
+			return -1;
+		}
+	} else {
+		if (field_offset < 0) {
+			zend_value_error("Argument #%d must be greater than or equal to 0", arg_num);
+			return -1;
+		}
+		if (field_offset >= PQnfields(result)) {
+			zend_value_error("Argument #%d must be less than the number of fields for this result set", arg_num);
+			return -1;
+		}
+	}
+	return field_offset;
+}
+
 /* {{{ Returns values from a result identifier */
 PHP_FUNCTION(pg_fetch_result)
 {
@@ -1777,21 +1800,10 @@ PHP_FUNCTION(pg_fetch_result)
 		}
 		pgsql_row = (int)row;
 	}
-	if (field_name) {
-		field_offset = PQfnumber(pgsql_result, ZSTR_VAL(field_name));
-		if (field_offset < 0 || field_offset >= PQnfields(pgsql_result)) {
-			php_error_docref(NULL, E_WARNING, "Bad column offset specified");
-			RETURN_FALSE;
-		}
-	} else {
-		if (field_offset < 0) {
-			zend_argument_value_error(argc, "must be greater than or equal to 0");
-			RETURN_THROWS();
-		}
-		if (field_offset >= PQnfields(pgsql_result)) {
-			php_error_docref(NULL, E_WARNING, "Bad column offset specified");
-			RETURN_FALSE;
-		}
+
+	field_offset = field_arg_to_offset(pgsql_result, field_name, field_offset, argc);
+	if (field_offset < 0) {
+		RETURN_THROWS();
 	}
 
 	if (PQgetisnull(pgsql_result, pgsql_row, field_offset)) {
@@ -2037,8 +2049,8 @@ PHP_FUNCTION(pg_fetch_all_columns)
 
 	num_fields = PQnfields(pgsql_result);
 	if (colno >= (zend_long)num_fields) {
-		php_error_docref(NULL, E_WARNING, "Invalid column number '" ZEND_LONG_FMT "'", colno);
-		RETURN_FALSE;
+		zend_argument_value_error(2, "must be less than the number of fields for this result set");
+		RETURN_THROWS();
 	}
 
 	array_init(return_value);
@@ -2134,20 +2146,9 @@ static void php_pgsql_data_info(INTERNAL_FUNCTION_PARAMETERS, int entry_type)
 		pgsql_row = (int)row;
 	}
 
-	if (field_name) {
-		field_offset = PQfnumber(pgsql_result, ZSTR_VAL(field_name));
-		if (field_offset < 0 || field_offset >= PQnfields(pgsql_result)) {
-			php_error_docref(NULL, E_WARNING, "Bad column offset specified");
-			RETURN_FALSE;
-		}
-	} else {
-		if (field_offset < 0) {
-			zend_argument_value_error(argc, "must be greater than or equal to 0");
-		}
-		if (field_offset >= PQnfields(pgsql_result)) {
-			php_error_docref(NULL, E_WARNING, "Bad column offset specified");
-			RETURN_FALSE;
-		}
+	field_offset = field_arg_to_offset(pgsql_result, field_name, field_offset, argc);
+	if (field_offset < 0) {
+		RETURN_THROWS();
 	}
 
 	switch (entry_type) {
diff --git a/ext/pgsql/pgsql.stub.php b/ext/pgsql/pgsql.stub.php
index 181d62c9cd..4e2b21e4f2 100644
--- a/ext/pgsql/pgsql.stub.php
+++ b/ext/pgsql/pgsql.stub.php
@@ -118,37 +118,37 @@ function pg_last_notice($connection, int $option = PGSQL_NOTICE_LAST): array|str
 function pg_field_table($result, int $field_number, bool $oid_only = false): string|int|false {}
 
 /** @param resource $result */
-function pg_field_name($result, int $field_number): string|false {}
+function pg_field_name($result, int $field_number): string {}
 
 /**
  * @param resource $result
  * @alias pg_field_name
  * @deprecated
  */
-function pg_fieldname($result, int $field_number): string|false {}
+function pg_fieldname($result, int $field_number): string {}
 
 /** @param resource $result */
-function pg_field_size($result, int $field_number): int|false {}
+function pg_field_size($result, int $field_number): int {}
 
 /**
  * @param resource $result
  * @alias pg_field_size
  * @deprecated
  */
-function pg_fieldsize($result, int $field_number): int|false {}
+function pg_fieldsize($result, int $field_number): int {}
 
 /** @param resource $result */
-function pg_field_type($result, int $field_number): string|false {}
+function pg_field_type($result, int $field_number): string {}
 
 /**
  * @param resource $result
  * @alias pg_field_type
  * @deprecated
  */
-function pg_fieldtype($result, int $field_number): string|false {}
+function pg_fieldtype($result, int $field_number): string {}
 
 /** @param resource $result */
-function pg_field_type_oid($result, int $field_number): string|int|false {}
+function pg_field_type_oid($result, int $field_number): string|int {}
 
 /** @param resource $result */
 function pg_field_num($result, string $field_name): int {}
@@ -196,7 +196,7 @@ function pg_fetch_object($result, ?int $row_number = null, string $class_name =
 function pg_fetch_all($result, int $result_type = PGSQL_ASSOC): array|false {}
 
 /** @param resource $result */
-function pg_fetch_all_columns($result, int $column_number = 0): array|false {}
+function pg_fetch_all_columns($result, int $field_number = 0): array {}
 
 /** @param resource $result */
 function pg_result_seek($result, int $row_number): bool {}
diff --git a/ext/pgsql/pgsql_arginfo.h b/ext/pgsql/pgsql_arginfo.h
index c2b2387154..f6cc2cf69c 100644
--- a/ext/pgsql/pgsql_arginfo.h
+++ b/ext/pgsql/pgsql_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 87152e947ab7bfb3a9d7df30dd6fbccac504504e */
+ * Stub hash: 3f5e097d572721b42f2ad438c2af8c0d1f9c9086 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_pg_connect, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, connection_string, IS_STRING, 0)
@@ -93,14 +93,14 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_pg_field_table, 0, 2, MAY_BE_STR
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, oid_only, _IS_BOOL, 0, "false")
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_pg_field_name, 0, 2, MAY_BE_STRING|MAY_BE_FALSE)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pg_field_name, 0, 2, IS_STRING, 0)
 	ZEND_ARG_INFO(0, result)
 	ZEND_ARG_TYPE_INFO(0, field_number, IS_LONG, 0)
 ZEND_END_ARG_INFO()
 
 #define arginfo_pg_fieldname arginfo_pg_field_name
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_pg_field_size, 0, 2, MAY_BE_LONG|MAY_BE_FALSE)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pg_field_size, 0, 2, IS_LONG, 0)
 	ZEND_ARG_INFO(0, result)
 	ZEND_ARG_TYPE_INFO(0, field_number, IS_LONG, 0)
 ZEND_END_ARG_INFO()
@@ -111,7 +111,7 @@ ZEND_END_ARG_INFO()
 
 #define arginfo_pg_fieldtype arginfo_pg_field_name
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_pg_field_type_oid, 0, 2, MAY_BE_STRING|MAY_BE_LONG|MAY_BE_FALSE)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_pg_field_type_oid, 0, 2, MAY_BE_STRING|MAY_BE_LONG)
 	ZEND_ARG_INFO(0, result)
 	ZEND_ARG_TYPE_INFO(0, field_number, IS_LONG, 0)
 ZEND_END_ARG_INFO()
@@ -160,9 +160,9 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_pg_fetch_all, 0, 1, MAY_BE_ARRAY
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, result_type, IS_LONG, 0, "PGSQL_ASSOC")
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_pg_fetch_all_columns, 0, 1, MAY_BE_ARRAY|MAY_BE_FALSE)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pg_fetch_all_columns, 0, 1, IS_ARRAY, 0)
 	ZEND_ARG_INFO(0, result)
-	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, column_number, IS_LONG, 0, "0")
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, field_number, IS_LONG, 0, "0")
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pg_result_seek, 0, 2, _IS_BOOL, 0)
diff --git a/ext/pgsql/tests/03sync_query.phpt b/ext/pgsql/tests/03sync_query.phpt
index 1cd439bc20..ce062cafd6 100644
--- a/ext/pgsql/tests/03sync_query.phpt
+++ b/ext/pgsql/tests/03sync_query.phpt
@@ -31,6 +31,32 @@ for ($i=0; $i < $rows; $i++)
     pg_fetch_result($result, $i, 0);
 }
 
+try {
+    pg_fetch_result($result, 0, -1);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_fetch_result($result, 0, 3);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_fetch_result($result, 0, "unknown");
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_fetch_all_columns($result, -1);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_fetch_all_columns($result, 3);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+
 pg_result_error($result);
 if (function_exists('pg_result_error_field')) {
     pg_result_error_field($result, PGSQL_DIAG_SEVERITY);
@@ -61,6 +87,42 @@ pg_field_type($result, 0);
 pg_field_prtlen($result, 0);
 pg_field_is_null($result, 0);
 
+try {
+    pg_field_is_null($result, -1);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_field_is_null($result, 3);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_field_is_null($result, "unknown");
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_field_name($result, -1);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_field_name($result, 3);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_field_table($result, -1);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    pg_field_table($result, 3);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+
 $result = pg_query($db, "INSERT INTO ".$table_name." VALUES (9999, 'ABC');");
 pg_last_oid($result);
 
@@ -70,4 +132,16 @@ pg_close($db);
 echo "OK";
 ?>
 --EXPECT--
+Argument #3 must be greater than or equal to 0
+Argument #3 must be less than the number of fields for this result set
+Argument #3 must be a field name from this result set
+pg_fetch_all_columns(): Argument #2 ($field_number) must be greater than or equal to 0
+pg_fetch_all_columns(): Argument #2 ($field_number) must be less than the number of fields for this result set
+Argument #2 must be greater than or equal to 0
+Argument #2 must be less than the number of fields for this result set
+Argument #2 must be a field name from this result set
+pg_field_name(): Argument #2 ($field_number) must be greater than or equal to 0
+pg_field_name(): Argument #2 ($field_number) must be less than the number of fields for this result set
+pg_field_table(): Argument #2 ($field_number) must be greater than or equal to 0
+pg_field_table(): Argument #2 ($field_number) must be less than the number of fields for this result set
 OK