From b839ede8784221aa09895e29b2bc91b1787482c3 Mon Sep 17 00:00:00 2001 From: Jenny Tam Date: Mon, 15 Jul 2019 14:21:54 -0700 Subject: [PATCH] Improve data caching with datetime objects (#1008) --- source/pdo_sqlsrv/pdo_stmt.cpp | 23 +++----- source/shared/core_sqlsrv.h | 2 + source/shared/core_stmt.cpp | 56 +++++++------------ source/shared/core_util.cpp | 34 +++++++++++ source/sqlsrv/stmt.cpp | 10 ++-- .../pdo_fetch_datetime_time_as_objects.phpt | 35 ++++++++++-- .../pdo_fetch_datetime_time_nulls.phpt | 1 - 7 files changed, 99 insertions(+), 62 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index edc5284d..9e7aee27 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -220,7 +220,7 @@ void meta_data_free( _Inout_ field_meta_data* meta ) sqlsrv_free( meta ); } -zval convert_to_zval( _In_ SQLSRV_PHPTYPE sqlsrv_php_type, _Inout_ void** in_val, _In_opt_ SQLLEN field_len ) +zval convert_to_zval(_Inout_ sqlsrv_stmt* stmt, _In_ SQLSRV_PHPTYPE sqlsrv_php_type, _Inout_ void** in_val, _In_opt_ SQLLEN field_len ) { zval out_zval; ZVAL_UNDEF(&out_zval); @@ -264,15 +264,8 @@ zval convert_to_zval( _In_ SQLSRV_PHPTYPE sqlsrv_php_type, _Inout_ void** in_val break; } case SQLSRV_PHPTYPE_DATETIME: - if (*in_val == NULL) { - - ZVAL_NULL(&out_zval); - } - else { - - out_zval = *(reinterpret_cast(*in_val)); - sqlsrv_free(*in_val); - } + convert_datetime_string_to_zval(stmt, static_cast(*in_val), field_len, out_zval); + sqlsrv_free(*in_val); break; case SQLSRV_PHPTYPE_NULL: ZVAL_NULL(&out_zval); @@ -833,11 +826,11 @@ int pdo_sqlsrv_stmt_get_col_data( _Inout_ pdo_stmt_t *stmt, _In_ int colno, core_sqlsrv_get_field( driver_stmt, colno, sqlsrv_php_type, false, *(reinterpret_cast(ptr)), reinterpret_cast( len ), true, &sqlsrv_phptype_out TSRMLS_CC ); - if ( ptr ) { - zval* zval_ptr = reinterpret_cast( sqlsrv_malloc( sizeof( zval ))); - *zval_ptr = convert_to_zval( sqlsrv_phptype_out, reinterpret_cast( ptr ), *len ); - *ptr = reinterpret_cast( zval_ptr ); - *len = sizeof( zval ); + if (ptr) { + zval* zval_ptr = reinterpret_cast(sqlsrv_malloc(sizeof(zval))); + *zval_ptr = convert_to_zval(driver_stmt, sqlsrv_phptype_out, reinterpret_cast(ptr), *len); + *ptr = reinterpret_cast(zval_ptr); + *len = sizeof(zval); } return 1; diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index bb7c8d5c..2caef510 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -1810,6 +1810,8 @@ bool validate_string( _In_ char* string, _In_ SQLLEN& len); bool convert_string_from_utf16( _In_ SQLSRV_ENCODING encoding, _In_reads_bytes_(cchInLen) const SQLWCHAR* inString, _In_ SQLINTEGER cchInLen, _Inout_updates_bytes_(cchOutLen) char** outString, _Out_ SQLLEN& cchOutLen ); SQLWCHAR* utf16_string_from_mbcs_string( _In_ SQLSRV_ENCODING php_encoding, _In_reads_bytes_(mbcs_len) const char* mbcs_string, _In_ unsigned int mbcs_len, _Out_ unsigned int* utf16_len, bool use_strict_conversion = false ); +void convert_datetime_string_to_zval(_Inout_ sqlsrv_stmt* stmt, _In_opt_ char* input, _In_ SQLLEN length, _Inout_ zval& out_zval); + //********************************************************************************************************************************* // Error handling routines and Predefined Errors //********************************************************************************************************************************* diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 87c58a36..faa909d4 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -1867,54 +1867,36 @@ void core_get_field_common( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT field_i break; } - // get the date as a string (http://msdn2.microsoft.com/en-us/library/ms712387(VS.85).aspx) and - // convert it to a DateTime object and return the created object + // Reference: https://docs.microsoft.com/sql/odbc/reference/appendixes/sql-to-c-timestamp + // Retrieve the datetime data as a string, which may be cached for later use. + // The string is converted to a DateTime object only when it is required to + // be returned as a zval. case SQLSRV_PHPTYPE_DATETIME: { - char field_value_temp[MAX_DATETIME_STRING_LEN] = {'\0'}; - zval params[1]; - zval field_value_temp_z; - zval function_z; + char* field_value_temp = NULL; + SQLLEN field_len_temp = 0; - ZVAL_UNDEF( &field_value_temp_z ); - ZVAL_UNDEF( &function_z ); - ZVAL_UNDEF( params ); + field_value_temp = static_cast(sqlsrv_malloc(MAX_DATETIME_STRING_LEN)); + memset(field_value_temp, '\0', MAX_DATETIME_STRING_LEN); - SQLRETURN r = stmt->current_results->get_data( field_index + 1, SQL_C_CHAR, field_value_temp, - MAX_DATETIME_STRING_LEN, field_len, true TSRMLS_CC ); + SQLRETURN r = stmt->current_results->get_data(field_index + 1, SQL_C_CHAR, field_value_temp, MAX_DATETIME_STRING_LEN, &field_len_temp, true TSRMLS_CC); - CHECK_CUSTOM_ERROR(( r == SQL_NO_DATA ), stmt, SQLSRV_ERROR_NO_DATA, field_index ) { + if (r == SQL_NO_DATA || field_len_temp == SQL_NULL_DATA) { + sqlsrv_free(field_value_temp); + field_value_temp = NULL; + field_len_temp = 0; + } + + CHECK_CUSTOM_ERROR((r == SQL_NO_DATA), stmt, SQLSRV_ERROR_NO_DATA, field_index) { throw core::CoreException(); } - zval_auto_ptr return_value_z; - return_value_z = ( zval * )sqlsrv_malloc( sizeof( zval )); - ZVAL_UNDEF( return_value_z ); + field_value = field_value_temp; + *field_len = field_len_temp; - if( *field_len == SQL_NULL_DATA ) { - ZVAL_NULL( return_value_z ); - field_value = reinterpret_cast( return_value_z.get()); - return_value_z.transferred(); - break; - } - - // Convert the string date to a DateTime object - core::sqlsrv_zval_stringl( &field_value_temp_z, field_value_temp, *field_len ); - core::sqlsrv_zval_stringl( &function_z, "date_create", sizeof("date_create") - 1 ); - params[0] = field_value_temp_z; - - if( call_user_function( EG( function_table ), NULL, &function_z, return_value_z, 1, - params TSRMLS_CC ) == FAILURE) { - THROW_CORE_ERROR(stmt, SQLSRV_ERROR_DATETIME_CONVERSION_FAILED); - } - - field_value = reinterpret_cast( return_value_z.get()); - return_value_z.transferred(); - zend_string_free( Z_STR( field_value_temp_z )); - zend_string_free( Z_STR( function_z )); break; } - + // create a stream wrapper around the field and return that object to the PHP script. calls to fread // on the stream will result in calls to SQLGetData. This is handled in stream.cpp. See that file // for how these fields are used. diff --git a/source/shared/core_util.cpp b/source/shared/core_util.cpp index e5427f97..92e9ba1d 100644 --- a/source/shared/core_util.cpp +++ b/source/shared/core_util.cpp @@ -189,6 +189,40 @@ SQLWCHAR* utf16_string_from_mbcs_string( _In_ SQLSRV_ENCODING php_encoding, _In_ return utf16_string; } +// Converts an input (assuming a datetime string) to a zval containing a PHP DateTime object. +// If the input is null, this simply returns a NULL zval. If anything wrong occurs during conversion, +// an exception will be thrown. +void convert_datetime_string_to_zval(_Inout_ sqlsrv_stmt* stmt, _In_opt_ char* input, _In_ SQLLEN length, _Inout_ zval& out_zval) +{ + if (input == NULL) { + ZVAL_NULL(&out_zval); + return; + } + + zval params[1]; + zval value_temp_z; + zval function_z; + + // Initialize all zval variables + ZVAL_UNDEF(&out_zval); + ZVAL_UNDEF(&value_temp_z); + ZVAL_UNDEF(&function_z); + ZVAL_UNDEF(params); + + // Convert the datetime string to a PHP DateTime object + core::sqlsrv_zval_stringl(&value_temp_z, input, length); + core::sqlsrv_zval_stringl(&function_z, "date_create", sizeof("date_create") - 1); + params[0] = value_temp_z; + + if (call_user_function(EG(function_table), NULL, &function_z, &out_zval, 1, + params TSRMLS_CC) == FAILURE) { + THROW_CORE_ERROR(stmt, SQLSRV_ERROR_DATETIME_CONVERSION_FAILED); + } + + zend_string_free(Z_STR(value_temp_z)); + zend_string_free(Z_STR(function_z)); +} + // call to retrieve an error from ODBC. This uses SQLGetDiagRec, so the // errno is 1 based. It returns it as an array with 3 members: // 1/SQLSTATE) sqlstate diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index da2d3c94..08ed482f 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -1513,11 +1513,11 @@ void convert_to_zval( _Inout_ sqlsrv_stmt* stmt, _In_ SQLSRV_PHPTYPE sqlsrv_php_ Z_TRY_ADDREF( out_zval ); break; } - case SQLSRV_PHPTYPE_DATETIME: - { - out_zval = *( static_cast( in_val )); - break; - } + case SQLSRV_PHPTYPE_DATETIME: + { + convert_datetime_string_to_zval(stmt, static_cast(in_val), field_len, out_zval); + break; + } case SQLSRV_PHPTYPE_NULL: ZVAL_NULL(&out_zval); diff --git a/test/functional/pdo_sqlsrv/pdo_fetch_datetime_time_as_objects.phpt b/test/functional/pdo_sqlsrv/pdo_fetch_datetime_time_as_objects.phpt index f83bc4c0..8463875b 100644 --- a/test/functional/pdo_sqlsrv/pdo_fetch_datetime_time_as_objects.phpt +++ b/test/functional/pdo_sqlsrv/pdo_fetch_datetime_time_as_objects.phpt @@ -70,6 +70,33 @@ function checkColumnDTValue($index, $column, $values, $dtObj) } } +function randomColumns($conn, $query, $columns, $values) +{ + $conn->setAttribute(PDO::SQLSRV_ATTR_FETCHES_DATETIME_TYPE, true); + $stmt = $conn->prepare($query); + + // Fetch a random column to trigger caching + $lastCol = count($columns) - 1; + $col = rand(0, $lastCol); + $stmt->execute(); + $dtObj = $stmt->fetchColumn($col); + checkColumnDTValue($col, $columns[$col], $values, $dtObj); + + // Similarly, fetch another column + $col = (++$col) % count($columns); + $stmt->execute(); + $dtObj = $stmt->fetchColumn($col); + checkColumnDTValue($col, $columns[$col], $values, $dtObj); + + // Now fetch all columns in a backward order + $i = $lastCol; + do { + $stmt->execute(); + $dtObj = $stmt->fetchColumn($i); + checkColumnDTValue($i, $columns[$i], $values, $dtObj); + } while (--$i >= 0); +} + function runTest($conn, $query, $columns, $values, $useBuffer = false) { // fetch the date time values as strings or date time objects @@ -103,7 +130,7 @@ function runTest($conn, $query, $columns, $values, $useBuffer = false) $stmt->execute(); $row = $stmt->fetch(PDO::FETCH_BOTH); checkDTObjectValues($row, $columns, $values, PDO::FETCH_BOTH); - + // ATTR_STRINGIFY_FETCHES should have no effect when fetching date time objects // Setting it to true only converts numeric values to strings when fetching // See http://www.php.net/manual/en/pdo.setattribute.php for details @@ -164,7 +191,6 @@ function runTest($conn, $query, $columns, $values, $useBuffer = false) // last test: set statement attribute fetch_datetime on with no change to // prepared statement -- expected datetime objects to be returned $stmt->setAttribute(PDO::SQLSRV_ATTR_FETCHES_DATETIME_TYPE, true); - $stmt->execute(); $i = 0; do { $stmt->execute(); @@ -234,8 +260,9 @@ try { $query = "SELECT * FROM $tableName"; - runTest($conn, $query, $columns, $values); - runTest($conn, $query, $columns, $values, true); + runtest($conn, $query, $columns, $values); + runtest($conn, $query, $columns, $values, true); + randomColumns($conn, $query, $columns, $values); dropTable($conn, $tableName); diff --git a/test/functional/pdo_sqlsrv/pdo_fetch_datetime_time_nulls.phpt b/test/functional/pdo_sqlsrv/pdo_fetch_datetime_time_nulls.phpt index 9bffdf75..98c7c957 100644 --- a/test/functional/pdo_sqlsrv/pdo_fetch_datetime_time_nulls.phpt +++ b/test/functional/pdo_sqlsrv/pdo_fetch_datetime_time_nulls.phpt @@ -110,7 +110,6 @@ function runTest($conn, $query, $columns, $useBuffer = false) // last test: set statement attribute fetch_datetime on with no change to // prepared statement -- expected datetime objects to be returned $stmt->setAttribute(PDO::SQLSRV_ATTR_FETCHES_DATETIME_TYPE, true); - $stmt->execute(); $i = 0; do { $stmt->execute();