Skip to content

Commit

Permalink
Merge pull request #744 from david-puglielli/Revert-bigint-change
Browse files Browse the repository at this point in the history
Reverted change handling bigint output parameters
  • Loading branch information
david-puglielli authored Apr 11, 2018
2 parents dd64980 + bbf951c commit 454bc27
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 88 deletions.
8 changes: 3 additions & 5 deletions source/shared/core_sqlsrv.h
Original file line number Diff line number Diff line change
Expand Up @@ -1315,11 +1315,10 @@ struct sqlsrv_output_param {
SQLUSMALLINT param_num; // used to index into the ind_or_len of the statement
SQLLEN original_buffer_len; // used to make sure the returned length didn't overflow the buffer
bool is_bool;
bool is_long;

// string output param constructor
sqlsrv_output_param( _In_ zval* p_z, _In_ SQLSRV_ENCODING enc, _In_ int num, _In_ SQLUINTEGER buffer_len, _In_ bool is_long ) :
param_z( p_z ), encoding( enc ), param_num( num ), original_buffer_len( buffer_len ), is_bool( false ), is_long( is_long )
sqlsrv_output_param( _In_ zval* p_z, _In_ SQLSRV_ENCODING enc, _In_ int num, _In_ SQLUINTEGER buffer_len ) :
param_z( p_z ), encoding( enc ), param_num( num ), original_buffer_len( buffer_len ), is_bool( false )
{
}

Expand All @@ -1329,8 +1328,7 @@ struct sqlsrv_output_param {
encoding( SQLSRV_ENCODING_INVALID ),
param_num( num ),
original_buffer_len( -1 ),
is_bool( is_bool ),
is_long( false )
is_bool( is_bool )
{
}
};
Expand Down
37 changes: 5 additions & 32 deletions source/shared/core_stmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ void core_sqlsrv_bind_param( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT param_
}
bool zval_was_null = ( Z_TYPE_P( param_z ) == IS_NULL );
bool zval_was_bool = ( Z_TYPE_P( param_z ) == IS_TRUE || Z_TYPE_P( param_z ) == IS_FALSE );
bool zval_was_long = ( Z_TYPE_P( param_z ) == IS_LONG && php_out_type == SQLSRV_PHPTYPE_INT && (sql_type == SQL_BIGINT || sql_type == SQL_UNKNOWN_TYPE ));
// if the user asks for for a specific type for input and output, make sure the data type we send matches the data we
// type we expect back, since we can only send and receive the same type. Anything can be converted to a string, so
// we always let that match if they want a string back.
Expand All @@ -383,16 +382,7 @@ void core_sqlsrv_bind_param( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT param_
if( zval_was_null || zval_was_bool ){
convert_to_long( param_z );
}
if( zval_was_long ){
convert_to_string( param_z );
if( encoding != SQLSRV_ENCODING_SYSTEM && encoding != SQLSRV_ENCODING_UTF8 && encoding != SQLSRV_ENCODING_BINARY ){
encoding = SQLSRV_ENCODING_SYSTEM;
}
match = Z_TYPE_P( param_z ) == IS_STRING;
}
else{
match = Z_TYPE_P( param_z ) == IS_LONG;
}
match = Z_TYPE_P( param_z ) == IS_LONG;
break;
case SQLSRV_PHPTYPE_FLOAT:
if( zval_was_null ){
Expand Down Expand Up @@ -431,15 +421,7 @@ void core_sqlsrv_bind_param( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT param_
if( direction == SQL_PARAM_OUTPUT ){
switch( php_out_type ) {
case SQLSRV_PHPTYPE_INT:
if( zval_was_long ){
convert_to_string( param_z );
if( encoding != SQLSRV_ENCODING_SYSTEM && encoding != SQLSRV_ENCODING_UTF8 && encoding != SQLSRV_ENCODING_BINARY ){
encoding = SQLSRV_ENCODING_SYSTEM;
}
}
else{
convert_to_long( param_z );
}
convert_to_long( param_z );
break;
case SQLSRV_PHPTYPE_FLOAT:
convert_to_double( param_z );
Expand Down Expand Up @@ -572,7 +554,7 @@ void core_sqlsrv_bind_param( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT param_

bool converted = convert_input_param_to_utf16( param_z, param_z );
CHECK_CUSTOM_ERROR( !converted, stmt, SQLSRV_ERROR_INPUT_PARAM_ENCODING_TRANSLATE,
param_num + 1, get_last_error_message() ){
param_num + 1, get_last_error_message() ){
throw core::CoreException();
}
buffer = Z_STRVAL_P( param_z );
Expand All @@ -583,10 +565,10 @@ void core_sqlsrv_bind_param( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT param_
// since this is an output string, assure there is enough space to hold the requested size and
// set all the variables necessary (param_z, buffer, buffer_len, and ind_ptr)
resize_output_buffer_if_necessary( stmt, param_z, param_num, encoding, c_type, sql_type, column_size, decimal_digits,
buffer, buffer_len TSRMLS_CC );
buffer, buffer_len TSRMLS_CC );

// save the parameter to be adjusted and/or converted after the results are processed
sqlsrv_output_param output_param( param_ref, encoding, param_num, static_cast<SQLUINTEGER>( buffer_len ), zval_was_long );
sqlsrv_output_param output_param( param_ref, encoding, param_num, static_cast<SQLUINTEGER>( buffer_len ) );

save_output_param_for_later( stmt, output_param TSRMLS_CC );

Expand Down Expand Up @@ -2151,15 +2133,6 @@ void finalize_output_parameters( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC )
else {
core::sqlsrv_zval_stringl(value_z, str, str_len);
}
if ( output_param->is_long ) {
zval* value_z_temp = ( zval * )sqlsrv_malloc( sizeof( zval ));
ZVAL_COPY( value_z_temp, value_z );
convert_to_double( value_z_temp );
if ( Z_DVAL_P( value_z_temp ) > INT_MIN && Z_DVAL_P( value_z_temp ) < INT_MAX ) {
convert_to_long( value_z );
}
sqlsrv_free( value_z_temp );
}
}
break;
case IS_LONG:
Expand Down
57 changes: 30 additions & 27 deletions test/functional/pdo_sqlsrv/pdo_ae_output_param_binary_size.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,25 @@ function testOutputBinary($inout)
$stmt = $conn->prepare($outSql);
trace("\nParam $pdoParamType with INOUT = $inout\n");

if ($inout && $pdoParamType != PDO::PARAM_STR) {
if ($inout && $pdoParamType == PDO::PARAM_STR) {
// Currently do not support getting binary as strings + INOUT param
// See VSO 2829 for details
continue;
}

if (!isAEConnected() && $pdoParamType == PDO::PARAM_INT) {
// Without AE, there is no possible way to specify
// binary encoding for this output param type,
// so a value like 'd' will be incorrectly
// interpreted as integer value 100 (its ASCII value).
// Skipping this because this use case is meaningless
// anyway.
// With AE, this output param type would have caused
// an exception
continue;
}

if ($inout) {
$paramType = $pdoParamType | PDO::PARAM_INPUT_OUTPUT;
} else {
$paramType = $pdoParamType;
Expand All @@ -101,14 +117,11 @@ function testOutputBinary($inout)
if ($pdoParamType == PDO::PARAM_STR || $pdoParamType == PDO::PARAM_LOB) {
$stmt->bindParam(1, $det, $paramType, $length, PDO::SQLSRV_ENCODING_BINARY);
$stmt->bindParam(2, $rand, $paramType, $length, PDO::SQLSRV_ENCODING_BINARY);
} elseif ($pdoParamType == PDO::PARAM_BOOL || $pdoParamType == PDO::PARAM_INT) {
} else {
$det = $rand = 0;
$stmt->bindParam(1, $det, $paramType, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE);
$stmt->bindParam(2, $rand, $paramType, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE);
} else {
$stmt->bindParam(1, $det, $paramType, $length);
$stmt->bindParam(2, $rand, $paramType, $length);
}
}

try {
$stmt->execute();
Expand All @@ -127,12 +140,8 @@ function testOutputBinary($inout)
printValues($errMsg, $det, $rand, $input0, $input1);
}
} else {
// $pdoParamType is PDO::PARAM_INT
// This only occurs without AE -- likely a rare use case
// With AE enabled, this would have caused an exception
if (strval($det) != $ord0 || strval($rand) != $ord1) {
printValues($errMsg, $det, $rand, $ord0, $ord1);
}
echo "Something went wrong. This should not have happened\n";
printValues($errMsg, $det, $rand, $ord0, $ord1);
}
} catch (PDOException $e) {
$message = $e->getMessage();
Expand All @@ -147,20 +156,13 @@ function testOutputBinary($inout)
}
} elseif ($pdoParamType == PDO::PARAM_BOOL || PDO::PARAM_INT) {
if (isAEConnected()) {
if ($pdoParamType == PDO::PARAM_INT) {
// Expected to fail with this message
$error = "String data, right truncated for output parameter";
$found = strpos($message, $error);
} else {
// PDO::PARAM_BOOL -
// Expected error 07006 with AE enabled:
// "Restricted data type attribute violation"
// The data value returned for a parameter bound as
// SQL_PARAM_INPUT_OUTPUT or SQL_PARAM_OUTPUT could not
// be converted to the data type identified by the
// ValueType argument in SQLBindParameter.
$found = strpos($message, $errors['07006']);
}
// Expected error 07006 with AE enabled:
// "Restricted data type attribute violation"
// The data value returned for a parameter bound as
// SQL_PARAM_INPUT_OUTPUT or SQL_PARAM_OUTPUT could not
// be converted to the data type identified by the
// ValueType argument in SQLBindParameter.
$found = strpos($message, $errors['07006']);
} else {
// When not AE enabled, expected to fail with something like this message
// "Implicit conversion from data type nvarchar(max) to binary is not allowed. Use the CONVERT function to run this query."
Expand All @@ -169,6 +171,7 @@ function testOutputBinary($inout)
$found = strpos($message, $error);
}
if ($found === false) {
print "Exception: $message\n";
printValues($errMsg, $det, $rand, $input0, $input1);
}
} else {
Expand Down Expand Up @@ -207,4 +210,4 @@ echo "Done\n";
unset($conn);
?>
--EXPECT--
Done
Done
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function testOutputChars($inout)
if ($found === false) {
printValues($errMsg, $det, $rand, $input0, $input1);
}
} elseif ($pdoParamType == PDO::PARAM_BOOL) {
} elseif ($pdoParamType == PDO::PARAM_BOOL || $pdoParamType == PDO::PARAM_INT) {
if (isAEConnected()) {
// Expected error 22003: "Numeric value out of range"
$found = strpos($message, $errors['22003']);
Expand Down
5 changes: 3 additions & 2 deletions test/functional/pdo_sqlsrv/pdo_ae_output_param_datetimes.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ function testOutputDatetimes($inout)
// streams cannot be specified as output parameters."
$found = strpos($message, $errors['IMSSP']);
} elseif (isAEConnected()) {
if ($pdoParamType == PDO::PARAM_BOOL) {
if ($pdoParamType != PDO::PARAM_STR) {
// Expected error 07006: "Restricted data type attribute violation"
// What does this error mean?
// The data value returned for a parameter bound as
Expand All @@ -159,14 +159,15 @@ function testOutputDatetimes($inout)
$found = strpos($message, $error);
}
} else {
if ($pdoParamType == PDO::PARAM_BOOL) {
if ($pdoParamType != PDO::PARAM_STR) {
$error = "Operand type clash: int is incompatible with $dataType";
} else {
$error = "Error converting data type nvarchar to $dataType";
}
$found = strpos($message, $error);
}
if ($found === false) {
print $message . PHP_EOL;
printValues($errMsg, $det, $rand, $inputValues);
}
}
Expand Down
13 changes: 4 additions & 9 deletions test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,9 @@ function compareIntegers($det, $rand, $inputValues, $pdoParamType)
if (is_string($det)) {
return (!compareFloats(floatval($det), $inputValues[0])
&& !compareFloats(floatval($rand), $inputValues[1]));
} elseif ($pdoParamType == PDO::PARAM_INT) {
$input0 = floor($inputValues[0]); // the positive float
$input1 = ceil($inputValues[1]); // the negative float

return ($det == $input0 && $rand == $input1);
} else {
// $pdoParamType == PDO::PARAM_BOOL
// Expect bool(true) or bool(false) depending on the rounded input values
// if $pdoParamType is PDO::PARAM_BOOL,
// expect bool(true) or bool(false) depending on the rounded input values
// But with AE enabled (aforementioned GitHub issue), the fetched values
// are floats instead, which should be fixed
$input0 = floor($inputValues[0]); // the positive float
Expand Down Expand Up @@ -195,9 +190,9 @@ function testOutputDecimals($inout)
if ($found === false) {
printValues($errMsg, $det, $rand, $inputValues);
}
} elseif (!isAEConnected() && $precision >= 16 && $pdoParamType == PDO::PARAM_BOOL) {
} elseif (!isAEConnected() && $precision >= 16) {
// When not AE enabled, large numbers are expected to
// fail when converting to booleans
// fail when converting to booleans / integers
$error = "Error converting data type $dataType to int";
$found = strpos($message, $error);
if ($found === false) {
Expand Down
7 changes: 0 additions & 7 deletions test/functional/pdo_sqlsrv/pdo_ae_output_param_floats.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,6 @@ function testOutputFloats($fetchNumeric, $inout)
// call stored procedure
$outSql = getCallProcSqlPlaceholders($spname, 2);
foreach ($pdoParamTypes as $pdoParamType) {
if ($pdoParamType == PDO::PARAM_INT && (strtoupper(substr(PHP_OS, 0, 3)) != 'WIN' || substr(PHP_VERSION, 0, 3) == "7.0")) {
// Bug 2876 in VSO: Linux or PHP 7.0 - when retrieving a float as OUTPUT
// or INOUT parameter with PDO::PARAM_INT, the returned values
// are always single digits, regardless of the original floats
continue;
}

$det = 0.0;
$rand = 0.0;
$stmt = $conn->prepare($outSql);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function testOutputNChars($inout)
if ($found === false) {
printValues($errMsg, $det, $rand, $input0, $input1);
}
} elseif ($pdoParamType == PDO::PARAM_BOOL) {
} elseif ($pdoParamType == PDO::PARAM_BOOL || $pdoParamType == PDO::PARAM_INT) {
if (isAEConnected()) {
// Expected error 22003: "Numeric value out of range"
$found = strpos($message, $errors['22003']);
Expand Down
4 changes: 2 additions & 2 deletions test/functional/pdo_sqlsrv/pdo_bigint_outparam.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ insertRow($conn, $tbname, array("c1_bigint" => 922337203685479936));
$outSql = "{CALL $spname (?)}";
$bigintOut = 0;
$stmt = $conn->prepare($outSql);
$stmt->bindParam(1, $bigintOut, PDO::PARAM_INT, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE);
$stmt->bindParam(1, $bigintOut, PDO::PARAM_STR, 32);
$stmt->execute();
printf("Large bigint output:\n" );
var_dump($bigintOut);
Expand All @@ -35,7 +35,7 @@ printf("\n");
// Call stored procedure with inout
$bigintOut = 0;
$stmt = $conn->prepare($outSql);
$stmt->bindParam(1, $bigintOut, PDO::PARAM_INT | PDO::PARAM_INPUT_OUTPUT, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE);
$stmt->bindParam(1, $bigintOut, PDO::PARAM_STR | PDO::PARAM_INPUT_OUTPUT, 2048);
$stmt->execute();
printf("Large bigint inout:\n" );
var_dump($bigintOut);
Expand Down
4 changes: 2 additions & 2 deletions test/functional/sqlsrv/sqlsrv_bigint_outparam.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ AE\insertRow($conn, $tbname, array("c1_bigint" => 922337203685479936));
// Call stored procedure with SQLSRV_PARAM_OUT
$outSql = "{CALL $spname (?)}";
$bigintOut = 0;
$stmt = sqlsrv_prepare($conn, $outSql, array(array(&$bigintOut, SQLSRV_PARAM_OUT)));
$stmt = sqlsrv_prepare($conn, $outSql, array(array(&$bigintOut, SQLSRV_PARAM_OUT, null, SQLSRV_SQLTYPE_BIGINT)));
sqlsrv_execute($stmt);
printf("Large bigint output:\n");
var_dump($bigintOut);
printf("\n");

// Call stored procedure with SQLSRV_PARAM_INOUT
$bigintOut = 0;
$stmt = sqlsrv_prepare($conn, $outSql, array(array(&$bigintOut, SQLSRV_PARAM_INOUT)));
$stmt = sqlsrv_prepare($conn, $outSql, array(array(&$bigintOut, SQLSRV_PARAM_INOUT, null, SQLSRV_SQLTYPE_VARCHAR(32))));
sqlsrv_execute($stmt);
printf("Large bigint inout:\n");
var_dump($bigintOut);
Expand Down

0 comments on commit 454bc27

Please sign in to comment.