Added select type conversion test #783
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #783 +/- ##
=======================================
Coverage 80.06% 80.06%
=======================================
Files 25 25
Lines 7323 7323
=======================================
Hits 5863 5863
Misses 1460 1460
Continue to review full report at Codecov.
|
source/shared/core_sqlsrv.h
Outdated
| @@ -174,7 +174,7 @@ const int SQL_SERVER_MAX_PRECISION = 38; | |||
| const int SQL_SERVER_MAX_TYPE_SIZE = 0; | |||
| const int SQL_SERVER_MAX_PARAMS = 2100; | |||
| // increase the maximum message length to accommodate for the long error returned for operand type clash | |||
| const int SQL_MAX_ERROR_MESSAGE_LENGTH = SQL_MAX_MESSAGE_LENGTH * 7; | |||
| const int SQL_MAX_ERROR_MESSAGE_LENGTH = SQL_MAX_MESSAGE_LENGTH * 8; | |||
There was a problem hiding this comment.
Maybe extend the comment above to explain why the error message length can be very long
| @@ -0,0 +1,302 @@ | |||
| --TEST-- | |||
| Test fetching data by conversion with CAST in the SELECT statement | |||
| --SKIPIF-- | |||
There was a problem hiding this comment.
It would be nice to add a DESCRIPTION section to give some examples of what this test is about
|
|
||
| for ($i = 0; $i < $numTypes; ++$i) { | ||
| $selectQuery[] = array(); | ||
| $colnamei = str_replace(array("(", ",", ")"), array("_", "_", ""), $dataTypes[$i]); |
There was a problem hiding this comment.
Please add a comment to show an example of a typical $dataTypes[$i] and what $colnamei will end up to be
|
|
||
| // Conversion matrix for SQL types, based on the conversion chart | ||
| // at https://www.microsoft.com/en-us/download/details.aspx?id=35834 | ||
| // i = implicit conversion |
There was a problem hiding this comment.
Is this the reference page(s) that justify(ies) why some casting work and some don't?
Please add this link to the DESCRIPTION section as well
| // @ = not applicable | ||
| // c = explicit CAST required | ||
| // m = misc | ||
| $conversionMatrix = array(array('@','i','i','i','i','i','i','i','i','i','i','e','e','e','e','i','i','x','x','i','i','i','i','i','i','i','i','i','i','i','e','e','e','i','i'),//binary |
There was a problem hiding this comment.
What does misc mean in this context?
There was a problem hiding this comment.
specialised xml conversions, no longer in the matrix in this test so I've removed it.
| if ($stmt == false) { | ||
| print_r(sqlsrv_errors()); | ||
| fatalError("sqlsrv_prepare failed\n"); | ||
| } |
There was a problem hiding this comment.
I remember fatalError() will also print sqlsrv errors, so to print them before calling fatalError() is redundant...
There was a problem hiding this comment.
As I recall, it prints only the message itself and not the code?
There was a problem hiding this comment.
do you really need the error codes?
| $convError = sqlsrv_errors(); | ||
|
|
||
| // if the non-AE conversion fails, certainly the AE conversion | ||
| // should fail but only with error 22018. |
There was a problem hiding this comment.
Please extend the comment to include the meaning of 22018 - Conversion not allowed - such that we don't need to scroll all the way up to know what it means
| } elseif ($stmtAE != false and $conversionMatrixAE[$i][$j] == 'x') { | ||
| fatalError("AE conversion succeeded, should have failed. i=$i, j=$j, v=$v\n"); | ||
| } elseif ($stmtAE != false and $conversionMatrixAE[$i][$j] == 'y') { | ||
| $row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_NUMERIC); |
There was a problem hiding this comment.
Suggestion: rearrange the above if...else block such that:
if (!$stmtAE) {
if ($conversionMatrixAE[$i][$j] == 'x') {
...
} else { // ($conversionMatrixAE[$i][$j] == 'y')
...
}
} else { // query succeeded
if ($conversionMatrixAE[$i][$j] == 'x') {
...
} elseif ($conversionMatrixAE[$i][$j] == 'y') {
...
}
}
| print_r($row[0]); | ||
| print_r($rowAE[0]); | ||
| fatalError("Test failed, values do not match\n"); | ||
| } |
There was a problem hiding this comment.
Suggestion not to call fatalError() here when a test case failed. Perhaps printing what fail is enough such that the rest of test cases will continue.
| } | ||
|
|
||
| $deleteQuery = "DELETE FROM $tableName"; | ||
| $stmt = sqlsrv_query($conn, $deleteQuery); |
There was a problem hiding this comment.
Please use TRUNCATE $tableName instead
From DOCS: "Removes all rows from a table or specified partitions of a table, without logging the individual row deletions. TRUNCATE TABLE is similar to the DELETE statement with no WHERE clause; however, TRUNCATE TABLE is faster and uses fewer system and transaction log resources."
This change is