Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverted change handling bigint output parameters #744

Merged
merged 4 commits into from
Apr 11, 2018
Merged

Reverted change handling bigint output parameters #744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2960738
Reverted change handling bigint output parameters
david-puglielli Apr 10, 2018
8c681f2
Updated failing tests
david-puglielli Apr 11, 2018
99a11c1
Fixed binary test
david-puglielli Apr 11, 2018
bbf951c
Minor fixes
david-puglielli Apr 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions source/shared/core_sqlsrv.h
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if $pdoParamType is not PARAM_STR or PARAM_BOOL? Is this else branch not hit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only five different PDO PARAM types, PARAM_NULL and PARAM_LOB will result in an exception, and now PARAM_INT will be skipped with non-AE. With AE, PARAM_INT will also result in an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I see

}
} 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
2 changes: 1 addition & 1 deletion test/functional/pdo_sqlsrv/pdo_ae_output_param_char_size.phpt
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion test/functional/pdo_sqlsrv/pdo_ae_output_param_nchar_size.phpt
View file
Open in desktop
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
View file
Open in desktop
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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps not 2048 but a size that's a bit larger than 20?

printf("Large bigint inout:\n" );
var_dump($bigintOut);
Expand Down
4 changes: 2 additions & 2 deletions test/functional/sqlsrv/sqlsrv_bigint_outparam.phpt
View file
Open in desktop
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