Rewrite PDO result binding

Instead of requiring the type to be determined in advance by the
describer function and then requiring get_col to return a buffer
of appropriate type, allow get_col to return an arbitrary zval.
See UPGRADING.INTERNALS for a more detailed description of the
change.

This makes the result fetching simpler, more efficient and more
flexible. The general possibility already existed via the special
PDO_PARAM_ZVAL type, but the usage was very inconvenient and/or
inefficient. Now it's possible to easily implement behavior like
"return int if it fits, otherwise string" and to avoid any kind
of complex management of temporary buffers.

This also fixes bug #40913 (our second highest voted bug of all
time, for some reason). PARAM_LOB result bindings will now
consistently return a stream resource, independently of the used
database driver.

I've tried my best to update all PDO drivers for this change, but
some of the changes may be broken, as I cannot test or even build
some of these drivers (in particular PDO dblib and PDO oci).
Fixes are appreciated -- a working CI setup would be even more
appreciated ;)

Author: nikic

NEWS

@@ -21,6 +21,10 @@ PHP                                                                        NEWS
 - OpenSSL:
   . Bump minimal OpenSSL version to 1.0.2. (Jakub Zelenka)
 
+- PDO:
+  . Fixed bug #40913 (PDO_MYSQL: PDO::PARAM_LOB does not bind to a stream for
+    fetching a BLOB). (Nikita)
+
 - PSpell:
   . Convert resource<pspell> to object \PSpell. (Sara)
   . Convert resource<pspell config> to object \PSPellConfig. (Sara)

UPGRADING

@@ -42,6 +42,10 @@ PHP 8.1 UPGRADE NOTES
 - PDO:
   . PDO::ATTR_STRINGIFY_FETCHES now also stringifies values of type bool to
     "0" or "1". Previously booleans were not stringified.
+  . Calling bindColumn() with PDO::PARAM_LOB (and assuming stringification is
+    not enabled) will now consistently bind a stream result, as documented.
+    Previously the result would be either a stream or a string depending on the
+    used database driver and the time the binding is performed.
 
 - PDO MySQL:
   . Integers and floats in result sets will now be returned using native PHP

UPGRADING.INTERNALS

@@ -25,3 +25,25 @@ PHP 8.1 INTERNALS UPGRADE NOTES
       configuration. If the an algorithm doesn't make use of any
       additional configuration, the argument is to be marked with
       ZEND_ATTRIBUTE_UNUSED.
+
+  b. ext/pdo
+    - The "preparer" callback now accepts a zend_string* instead of
+      char* + size_t pair the query string. Similarly, the query_string and
+      active_query_string members of pdo_stmt_t are now zend_string*.
+    - The way in which drivers provide results has changed: Previously,
+      the "describer" callback populated the "pdo_type" member in the
+      pdo_column_data structure, and the "get_col" callback then had to return
+      pointers to data of appropriate type.
+
+      In PHP 8.1, the "describer" callback no longer determines the pdo_type
+      (and this member has been removed from pdo_column_data). Instead, the
+      "get_col" callback accepts a zval pointer that may be populated with a
+      value of arbitrary type. This gives drivers more flexibility in
+      determining result types (e.g. based on whether a specific integer fits
+      the PHP integer type) and avoid awkward juggling of temporary buffers.
+
+      As the "describer" no longer determines pdo_type, the "get_column_meta"
+      function is now responsible for providing this information for use by
+      getColumnMeta(). The type provided here does not need to match the type
+      returned by get_col (in fact no corresponding type might exist, e.g. for
+      floats). It should be the closest logical equivalent for the column type.

ext/pdo/pdo_stmt.c

@@ -486,14 +486,8 @@ PHP_METHOD(PDOStatement, execute)
 }
 /* }}} */
 
-static inline void fetch_value(pdo_stmt_t *stmt, zval *dest, int colno, int *type_override) /* {{{ */
+static inline void fetch_value(pdo_stmt_t *stmt, zval *dest, int colno, enum pdo_param_type *type_override) /* {{{ */
 {
-	struct pdo_column_data *col;
-	char *value = NULL;
-	size_t value_len = 0;
-	int caller_frees = 0;
-	int type, new_type;
-
 	if (colno < 0) {
 		zend_value_error("Column index must be greater than or equal to 0");
 		ZVAL_NULL(dest);
@@ -506,126 +500,62 @@ static inline void fetch_value(pdo_stmt_t *stmt, zval *dest, int colno, int *typ
 		return;
 	}
 
-	col = &stmt->columns[colno];
-	type = PDO_PARAM_TYPE(col->param_type);
-	new_type =  type_override ? (int)PDO_PARAM_TYPE(*type_override) : type;
-
-	value = NULL;
-	value_len = 0;
+	ZVAL_NULL(dest);
+	stmt->methods->get_col(stmt, colno, dest, type_override);
 
-	stmt->methods->get_col(stmt, colno, &value, &value_len, &caller_frees);
-
-	switch (type) {
-		case PDO_PARAM_ZVAL:
-			if (value && value_len == sizeof(zval)) {
-				ZVAL_COPY_VALUE(dest, (zval *)value);
-
-				if (Z_TYPE_P(dest) == IS_STRING && Z_STRLEN_P(dest) == 0
-						&& stmt->dbh->oracle_nulls == PDO_NULL_EMPTY_STRING) {
-					zval_ptr_dtor_str(dest);
-					ZVAL_NULL(dest);
-				}
-			} else {
-				ZVAL_NULL(dest);
-			}
+	if (Z_TYPE_P(dest) == IS_STRING && Z_STRLEN_P(dest) == 0
+			&& stmt->dbh->oracle_nulls == PDO_NULL_EMPTY_STRING) {
+		zval_ptr_dtor_str(dest);
+		ZVAL_NULL(dest);
+	}
 
-			if (Z_TYPE_P(dest) == IS_NULL) {
-				type = new_type;
-			}
-			break;
+	/* If stringification is requested, override with PDO_PARAM_STR. */
+	enum pdo_param_type pdo_param_str = PDO_PARAM_STR;
+	if (stmt->dbh->stringify) {
+		type_override = &pdo_param_str;
+	}
 
-		case PDO_PARAM_INT:
-			if (value && value_len == sizeof(zend_long)) {
-				ZVAL_LONG(dest, *(zend_long*)value);
+	if (type_override && Z_TYPE_P(dest) != IS_NULL) {
+		switch (*type_override) {
+			case PDO_PARAM_INT:
+				convert_to_long(dest);
 				break;
-			}
-			ZVAL_NULL(dest);
-			break;
-
-		case PDO_PARAM_BOOL:
-			if (value && value_len == sizeof(zend_bool)) {
-				ZVAL_BOOL(dest, *(zend_bool*)value);
+			case PDO_PARAM_BOOL:
+				convert_to_boolean(dest);
 				break;
-			}
-			ZVAL_NULL(dest);
-			break;
-
-		case PDO_PARAM_LOB:
-			if (value == NULL) {
-				ZVAL_NULL(dest);
-			} else if (value_len == 0) {
-				/* Warning, empty strings need to be passed as stream */
-				if (stmt->dbh->stringify || new_type == PDO_PARAM_STR) {
-					zend_string *buf;
-					buf = php_stream_copy_to_mem((php_stream*)value, PHP_STREAM_COPY_ALL, 0);
-					if (buf == NULL) {
+			case PDO_PARAM_STR:
+				if (Z_TYPE_P(dest) == IS_FALSE) {
+					/* Return "0" rather than "", because this is what database drivers that
+					 * don't have a dedicated boolean type would return. */
+					zval_ptr_dtor_nogc(dest);
+					ZVAL_INTERNED_STR(dest, ZSTR_CHAR('0'));
+				} else if (Z_TYPE_P(dest) == IS_RESOURCE) {
+					/* Convert LOB stream to string */
+					php_stream *stream;
+					php_stream_from_zval_no_verify(stream, dest);
+					zend_string *str = php_stream_copy_to_mem(stream, PHP_STREAM_COPY_ALL, 0);
+					zval_ptr_dtor_nogc(dest);
+					if (str == NULL) {
 						ZVAL_EMPTY_STRING(dest);
 					} else {
-						ZVAL_STR(dest, buf);
+						ZVAL_STR(dest, str);
 					}
-					php_stream_close((php_stream*)value);
-				} else {
-					php_stream_to_zval((php_stream*)value, dest);
-				}
-			} else if (!stmt->dbh->stringify && new_type != PDO_PARAM_STR) {
-				/* they gave us a string, but LOBs are represented as streams in PDO */
-				zend_string *str = zend_string_init(value, value_len, 0);
-				php_stream *stream = php_stream_memory_open(TEMP_STREAM_READONLY, str);
-				if (stream) {
-					php_stream_to_zval(stream, dest);
 				} else {
-					ZVAL_NULL(dest);
+					convert_to_string(dest);
 				}
-				zend_string_release(str);
-			} else {
-				ZVAL_STRINGL(dest, value, value_len);
-			}
-			break;
-
-		case PDO_PARAM_STR:
-			if (value && !(value_len == 0 && stmt->dbh->oracle_nulls == PDO_NULL_EMPTY_STRING)) {
-				ZVAL_STRINGL(dest, value, value_len);
-				break;
-			}
-		default:
-			ZVAL_NULL(dest);
-	}
-
-	if (type != new_type) {
-		switch (new_type) {
-			case PDO_PARAM_INT:
-				convert_to_long_ex(dest);
-				break;
-			case PDO_PARAM_BOOL:
-				convert_to_boolean_ex(dest);
-				break;
-			case PDO_PARAM_STR:
-				convert_to_string_ex(dest);
 				break;
 			case PDO_PARAM_NULL:
-				convert_to_null_ex(dest);
+				convert_to_null(dest);
 				break;
-			default:
-				;
-		}
-	}
-
-	if (caller_frees && value) {
-		efree(value);
-	}
-
-	if (stmt->dbh->stringify) {
-		switch (Z_TYPE_P(dest)) {
-			case IS_FALSE:
-				/* Return "0" rather than "", because this is what database drivers that
-				 * don't have a dedicated boolean type would return. */
-				zval_ptr_dtor_nogc(dest);
-				ZVAL_INTERNED_STR(dest, ZSTR_CHAR('0'));
+			case PDO_PARAM_LOB:
+				if (Z_TYPE_P(dest) == IS_STRING) {
+					php_stream *stream =
+						php_stream_memory_open(TEMP_STREAM_READONLY, Z_STR_P(dest));
+					zval_ptr_dtor_str(dest);
+					php_stream_to_zval(stream, dest);
+				}
 				break;
-			case IS_TRUE:
-			case IS_LONG:
-			case IS_DOUBLE:
-				convert_to_string(dest);
+			default:
 				break;
 		}
 	}
@@ -673,7 +603,7 @@ static bool do_fetch_common(pdo_stmt_t *stmt, enum pdo_fetch_orientation ori, ze
 				zval_ptr_dtor(Z_REFVAL(param->parameter));
 
 				/* set new value */
-				fetch_value(stmt, Z_REFVAL(param->parameter), param->paramno, (int *)&param->param_type);
+				fetch_value(stmt, Z_REFVAL(param->parameter), param->paramno, &param->param_type);
 
 				/* TODO: some smart thing that avoids duplicating the value in the
 				 * general loop below.  For now, if you're binding output columns,
@@ -1761,10 +1691,6 @@ PHP_METHOD(PDOStatement, getColumnMeta)
 	add_assoc_str(return_value, "name", zend_string_copy(col->name));
 	add_assoc_long(return_value, "len", col->maxlen); /* FIXME: unsigned ? */
 	add_assoc_long(return_value, "precision", col->precision);
-	if (col->param_type != PDO_PARAM_ZVAL) {
-		/* if param_type is PDO_PARAM_ZVAL the driver has to provide correct data */
-		add_assoc_long(return_value, "pdo_type", col->param_type);
-	}
 }
 /* }}} */
 

ext/pdo/php_pdo_driver.h

@@ -46,32 +46,14 @@ PDO_API char *php_pdo_int64_to_str(pdo_int64_t i64);
 
 enum pdo_param_type {
 	PDO_PARAM_NULL,
-
-	/* int as in long (the php native int type).
-	 * If you mark a column as an int, PDO expects get_col to return
-	 * a pointer to a long */
+	PDO_PARAM_BOOL,
 	PDO_PARAM_INT,
-
-	/* get_col ptr should point to start of the string buffer */
 	PDO_PARAM_STR,
-
-	/* get_col: when len is 0 ptr should point to a php_stream *,
-	 * otherwise it should behave like a string. Indicate a NULL field
-	 * value by setting the ptr to NULL */
 	PDO_PARAM_LOB,
 
-	/* get_col: will expect the ptr to point to a new PDOStatement object handle,
-	 * but this isn't wired up yet */
+	/* get_col: Not supported (yet?) */
 	PDO_PARAM_STMT, /* hierarchical result set */
 
-	/* get_col ptr should point to a zend_bool */
-	PDO_PARAM_BOOL,
-
-	/* get_col ptr should point to a zval*
-	   and the driver is responsible for adding correct type information to get_column_meta()
-	 */
-	PDO_PARAM_ZVAL,
-
 	/* magic flag to denote a parameter as being input/output */
 	PDO_PARAM_INPUT_OUTPUT = 0x80000000,
 
@@ -343,13 +325,13 @@ typedef int (*pdo_stmt_fetch_func)(pdo_stmt_t *stmt,
  * Driver should populate stmt->columns[colno] with appropriate info */
 typedef int (*pdo_stmt_describe_col_func)(pdo_stmt_t *stmt, int colno);
 
-/* retrieves pointer and size of the value for a column.
- * Note that PDO expects the driver to manage the lifetime of this data;
- * it will copy the value into a zval on behalf of the script.
- * If the driver sets caller_frees, ptr should point to emalloc'd memory
- * and PDO will free it as soon as it is done using it.
- */
-typedef int (*pdo_stmt_get_col_data_func)(pdo_stmt_t *stmt, int colno, char **ptr, size_t *len, int *caller_frees);
+/* Retrieves zval value of a column. If type is non-NULL, then this specifies the type which
+ * the user requested through bindColumn(). The driver does not need to check this argument,
+ * as PDO will perform any necessary conversions itself. However, it might be used to fetch
+ * a value more efficiently into the final type, or make the behavior dependent on the requested
+ * type. */
+typedef int (*pdo_stmt_get_col_data_func)(
+	pdo_stmt_t *stmt, int colno, zval *result, enum pdo_param_type *type);
 
 /* hook for bound params */
 enum pdo_param_event {
@@ -382,8 +364,8 @@ typedef int (*pdo_stmt_get_attr_func)(pdo_stmt_t *stmt, zend_long attr, zval *va
  *   name => the column name
  *   len => the length/size of the column
  *   precision => precision of the column
- *   pdo_type => an integer, one of the PDO_PARAM_XXX values
  *
+ *   pdo_type => an integer, one of the PDO_PARAM_XXX values
  *   scale => the floating point scale
  *   table => the table for that column
  *   type => a string representation of the type, mapped to the PHP equivalent type name
@@ -541,7 +523,6 @@ struct pdo_column_data {
 	zend_string *name;
 	size_t maxlen;
 	zend_ulong precision;
-	enum pdo_param_type param_type;
 };
 
 /* describes a bound parameter */

ext/pdo/tests/debug_emulated_prepares.phpt

@@ -47,17 +47,17 @@ Key: Name: [5] :bool
 paramno=-1
 name=[5] ":bool"
 is_param=1
-param_type=5
+param_type=1
 Key: Name: [4] :int
 paramno=-1
 name=[4] ":int"
 is_param=1
-param_type=1
+param_type=2
 Key: Name: [7] :string
 paramno=-1
 name=[7] ":string"
 is_param=1
-param_type=2
+param_type=3
 Key: Name: [5] :null
 paramno=-1
 name=[5] ":null"