Make ext/odbc default value handling more consistent (#13910)

These changes are carved off from https://github.com/php/php-src/pull/12040/files. I noticed that there are some inconsistencies between odbc_fetch_object()/odbc_fetch_array(), odbc_fetch_into(), as well as odbc_fetch_row(), specifically in how they handle the $row parameter. Now, I tried to align their behaviour the following way:

- I made null the default value. Previously, the default values were one of the following: -1, -1, 0, and null, respectively.
- odbc_fetch_row() has been returning false indicating there is no more rows when 0 is passed as $row. Now, a warning is also emitted in this case, because the null default value is not new, because it's available since PHP 8.0.
- When HAVE_SQL_EXTENDED_FETCH is not defined, the $row parameter is always ignored. Previously, some of the functions didn't accept it at all. Now a warning is emitted if the feature is not supported, but the parameter has any meaningful value (is greater than or equal to 1).

UPGRADING

@@ -64,6 +64,10 @@ PHP 8.4 UPGRADE NOTES
     is converted to Unicode. This is significant because around 40 SJIS-Mac characters
     convert to a sequence of multiple Unicode codepoints.
 
+- ODBC:
+  . odbc_fetch_row() returns false when a value less than or equal to 0 is
+    passed for parameter $row. Now, a warning is emitted in this case.
+
 - Opcache:
   . The JIT config defaults changed from opcache.jit=tracing and
     opcache.jit_buffer_size=0 to opcache.jit=disable and
@@ -360,6 +364,12 @@ PHP 8.4 UPGRADE NOTES
   . New serial_hex parameter added to openssl_csr_sign to allow setting serial
     number in the hexadecimal format.
 
+- ODBC:
+  . Parameter $row of odbc_fetch_object(), odbc_fetch_array(), and
+    odbc_fetch_into() now has a default value of null, consistent with
+    odbc_fetch_row(). Previously, the default values were -1, -1, and 0,
+    respectively.
+
 - Output:
   . Output handler status flags passed to the flags parameter of ob_start
     are now cleared.

ext/odbc/odbc.stub.php

@@ -352,17 +352,17 @@ function odbc_do($odbc, string $query) {}
 
 #ifdef PHP_ODBC_HAVE_FETCH_HASH
 /** @param resource $statement */
-function odbc_fetch_object($statement, int $row = -1): stdClass|false {}
+function odbc_fetch_object($statement, ?int $row = null): stdClass|false {}
 
 /** @param resource $statement */
-function odbc_fetch_array($statement, int $row = -1): array|false {}
+function odbc_fetch_array($statement, ?int $row = null): array|false {}
 #endif
 
 /**
  * @param resource $statement
  * @param array $array
  */
-function odbc_fetch_into($statement, &$array, int $row = 0): int|false {}
+function odbc_fetch_into($statement, &$array, ?int $row = null): int|false {}
 
 /** @param resource $statement */
 function odbc_fetch_row($statement, ?int $row = null): bool {}

ext/odbc/odbc_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 10479c33da6a72181c550c6690990e419d0dd5ad */
+ * Stub hash: a64be64f69159d0c8ad2c3b951c6451a040c3c73 */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_odbc_close_all, 0, 0, IS_VOID, 0)
 ZEND_END_ARG_INFO()
@@ -42,21 +42,21 @@ ZEND_END_ARG_INFO()
 #if defined(PHP_ODBC_HAVE_FETCH_HASH)
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_odbc_fetch_object, 0, 1, stdClass, MAY_BE_FALSE)
 	ZEND_ARG_INFO(0, statement)
-	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, row, IS_LONG, 0, "-1")
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, row, IS_LONG, 1, "null")
 ZEND_END_ARG_INFO()
 #endif
 
 #if defined(PHP_ODBC_HAVE_FETCH_HASH)
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_odbc_fetch_array, 0, 1, MAY_BE_ARRAY|MAY_BE_FALSE)
 	ZEND_ARG_INFO(0, statement)
-	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, row, IS_LONG, 0, "-1")
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, row, IS_LONG, 1, "null")
 ZEND_END_ARG_INFO()
 #endif
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_odbc_fetch_into, 0, 2, MAY_BE_LONG|MAY_BE_FALSE)
 	ZEND_ARG_INFO(0, statement)
 	ZEND_ARG_INFO(1, array)
-	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, row, IS_LONG, 0, "0")
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, row, IS_LONG, 1, "null")
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_odbc_fetch_row, 0, 1, _IS_BOOL, 0)

ext/odbc/php_odbc.c

@@ -1269,30 +1269,30 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type)
 	RETCODE rc;
 	SQLSMALLINT sql_c_type;
 	char *buf = NULL;
+	zend_long pv_row = 0;
+	bool pv_row_is_null = true;
+	zval *pv_res, tmp;
 #ifdef HAVE_SQL_EXTENDED_FETCH
 	SQLULEN crow;
 	SQLUSMALLINT RowStatus[1];
-	SQLLEN rownum;
-	zval *pv_res, tmp;
-	zend_long pv_row = -1;
-
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "r|l", &pv_res, &pv_row) == FAILURE) {
-		RETURN_THROWS();
-	}
-
-	rownum = pv_row;
-#else
-	zval *pv_res, tmp;
-
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "r", &pv_res) == FAILURE) {
-		RETURN_THROWS();
-	}
 #endif
 
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "r|l!", &pv_res, &pv_row, &pv_row_is_null) == FAILURE) {
+		RETURN_THROWS();
+	}
+
 	if ((result = (odbc_result *)zend_fetch_resource(Z_RES_P(pv_res), "ODBC result", le_result)) == NULL) {
 		RETURN_THROWS();
 	}
 
+	/* TODO deprecate $row argument values less than 1 after PHP 8.4 */
+
+#ifndef HAVE_SQL_EXTENDED_FETCH
+	if (!pv_row_is_null && pv_row > 0) {
+		php_error_docref(NULL, E_WARNING, "Extended fetch functionality is not available, argument #3 ($row) is ignored");
+	}
+#endif
+
 	if (result->numcols == 0) {
 		php_error_docref(NULL, E_WARNING, "No tuples available at this result index");
 		RETURN_FALSE;
@@ -1300,8 +1300,8 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type)
 
 #ifdef HAVE_SQL_EXTENDED_FETCH
 	if (result->fetch_abs) {
-		if (rownum > 0) {
-			rc = SQLExtendedFetch(result->stmt,SQL_FETCH_ABSOLUTE,rownum,&crow,RowStatus);
+		if (!pv_row_is_null && pv_row > 0) {
+			rc = SQLExtendedFetch(result->stmt,SQL_FETCH_ABSOLUTE,(SQLLEN)pv_row,&crow,RowStatus);
 		} else {
 			rc = SQLExtendedFetch(result->stmt,SQL_FETCH_NEXT,1,&crow,RowStatus);
 		}
@@ -1316,8 +1316,8 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type)
 	array_init(return_value);
 
 #ifdef HAVE_SQL_EXTENDED_FETCH
-	if (rownum > 0 && result->fetch_abs)
-		result->fetched = rownum;
+	if (!pv_row_is_null && pv_row > 0 && result->fetch_abs)
+		result->fetched = (SQLLEN)pv_row;
 	else
 #endif
 		result->fetched++;
@@ -1430,29 +1430,29 @@ PHP_FUNCTION(odbc_fetch_into)
 	SQLSMALLINT sql_c_type;
 	char *buf = NULL;
 	zval *pv_res, *pv_res_arr, tmp;
-#ifdef HAVE_SQL_EXTENDED_FETCH
 	zend_long pv_row = 0;
+	bool pv_row_is_null = true;
+#ifdef HAVE_SQL_EXTENDED_FETCH
 	SQLULEN crow;
 	SQLUSMALLINT RowStatus[1];
-	SQLLEN rownum = -1;
 #endif /* HAVE_SQL_EXTENDED_FETCH */
 
-#ifdef HAVE_SQL_EXTENDED_FETCH
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rz|l", &pv_res, &pv_res_arr, &pv_row) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rz|l!", &pv_res, &pv_res_arr, &pv_row, &pv_row_is_null) == FAILURE) {
 		RETURN_THROWS();
 	}
 
-	rownum = pv_row;
-#else
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rz", &pv_res, &pv_res_arr) == FAILURE) {
-		RETURN_THROWS();
-	}
-#endif /* HAVE_SQL_EXTENDED_FETCH */
-
 	if ((result = (odbc_result *)zend_fetch_resource(Z_RES_P(pv_res), "ODBC result", le_result)) == NULL) {
 		RETURN_THROWS();
 	}
 
+	/* TODO deprecate $row argument values less than 1 after PHP 8.4 */
+
+#ifndef HAVE_SQL_EXTENDED_FETCH
+	if (!pv_row_is_null && pv_row > 0) {
+		php_error_docref(NULL, E_WARNING, "Extended fetch functionality is not available, argument #3 ($row) is ignored");
+	}
+#endif
+
 	if (result->numcols == 0) {
 		php_error_docref(NULL, E_WARNING, "No tuples available at this result index");
 		RETURN_FALSE;
@@ -1465,8 +1465,8 @@ PHP_FUNCTION(odbc_fetch_into)
 
 #ifdef HAVE_SQL_EXTENDED_FETCH
 	if (result->fetch_abs) {
-		if (rownum > 0) {
-			rc = SQLExtendedFetch(result->stmt,SQL_FETCH_ABSOLUTE,rownum,&crow,RowStatus);
+		if (!pv_row_is_null && pv_row > 0) {
+			rc = SQLExtendedFetch(result->stmt,SQL_FETCH_ABSOLUTE,(SQLLEN)pv_row,&crow,RowStatus);
 		} else {
 			rc = SQLExtendedFetch(result->stmt,SQL_FETCH_NEXT,1,&crow,RowStatus);
 		}
@@ -1479,8 +1479,8 @@ PHP_FUNCTION(odbc_fetch_into)
 	}
 
 #ifdef HAVE_SQL_EXTENDED_FETCH
-	if (rownum > 0 && result->fetch_abs)
-		result->fetched = rownum;
+	if (!pv_row_is_null && pv_row > 0 && result->fetch_abs)
+		result->fetched = (SQLLEN)pv_row;
 	else
 #endif
 		result->fetched++;
@@ -1560,8 +1560,8 @@ PHP_FUNCTION(odbc_fetch_row)
 	odbc_result *result;
 	RETCODE rc;
 	zval *pv_res;
-	zend_long pv_row;
-	bool pv_row_is_null = 1;
+	zend_long pv_row = 0;
+	bool pv_row_is_null = true;
 #ifdef HAVE_SQL_EXTENDED_FETCH
 	SQLULEN crow;
 	SQLUSMALLINT RowStatus[1];
@@ -1575,6 +1575,17 @@ PHP_FUNCTION(odbc_fetch_row)
 		RETURN_THROWS();
 	}
 
+#ifndef HAVE_SQL_EXTENDED_FETCH
+	if (!pv_row_is_null) {
+		php_error_docref(NULL, E_WARNING, "Extended fetch functionality is not available, argument #3 ($row) is ignored");
+	}
+#else
+	if (!pv_row_is_null && pv_row < 1) {
+		php_error_docref(NULL, E_WARNING, "Argument #3 ($row) must be greater than or equal to 1");
+		RETURN_FALSE;
+	}
+#endif
+
 	if (result->numcols == 0) {
 		php_error_docref(NULL, E_WARNING, "No tuples available at this result index");
 		RETURN_FALSE;
@@ -1594,12 +1605,12 @@ PHP_FUNCTION(odbc_fetch_row)
 	if (rc != SQL_SUCCESS && rc != SQL_SUCCESS_WITH_INFO) {
 		RETURN_FALSE;
 	}
-
+#ifdef HAVE_SQL_EXTENDED_FETCH
 	if (!pv_row_is_null) {
 		result->fetched = (SQLLEN)pv_row;
-	} else {
+	} else
+#endif
 		result->fetched++;
-	}
 
 	RETURN_TRUE;
 }

ext/odbc/tests/odbc_fetch_row_001.phpt

@@ -40,6 +40,7 @@ $conn = odbc_connect($dsn, $user, $pass);
 odbc_exec($conn, 'DROP TABLE fetch_row');
 ?>
 --EXPECTF--
+Warning: odbc_fetch_row(): Argument #3 ($row) must be greater than or equal to 1 in %s on line %d
 bool(false)
 bool(true)
 string(1) "1"