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.

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

Test: Streaming with char encoding #262

Merged
merged 7 commits into from
Jan 31, 2017

Conversation

v-kigos
Copy link
Contributor

@v-kigos v-kigos commented Jan 28, 2017

No description provided.

@v-kigos v-kigos requested a review from yitam January 28, 2017 01:13
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 37.161% when pulling 144ad43 on v-kigos:PHP-7.0-Linux into 78598fc on Microsoft:PHP-7.0-Linux.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 37.71% when pulling f4cd8e3 on v-kigos:PHP-7.0-Linux into 78598fc on Microsoft:PHP-7.0-Linux.


// Connect
$conn = sqlsrv_connect( $serverName, $connectionInfo);
if( !$conn ) { die( print_r( sqlsrv_errors(), true)); }
Copy link
Contributor

@yitam yitam Jan 30, 2017

Choose a reason for hiding this comment

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

Would you consider creating a helper method for error handling (die(print_r(sqlsrv_errors...))? Since you create a database, shouldn't you drop database somewhere when errors occur?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if( !$conn ) { die( print_r( sqlsrv_errors(), true)); }

// Create database
sqlsrv_query($conn,"CREATE DATABASE ". $dbName) ?: die();
Copy link
Contributor

Choose a reason for hiding this comment

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

again, is a new database necessary?

$stmt = sqlsrv_query($conn, $query) ?: die( print_r( sqlsrv_errors(), true) );

// Fetch data
$query = "SELECT * FROM ".$tableName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a suggestion: $query = "SELECT * FROM $tableName";

sqlsrv_query($conn,"CREATE DATABASE ". $dbName) ?: die();

// Create table
$query = "CREATE TABLE ".$tableName." (ID NVARCHAR(10))";
Copy link
Contributor

@yitam yitam Jan 30, 2017

Choose a reason for hiding this comment

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

Is there any reason for NVARCHAR when no unicode character is used? Or consider adding a Unicode input value?

$stmt = sqlsrv_query( $conn, $query ) ?: die( print_r( sqlsrv_errors(), true) );

while(sqlsrv_fetch($stmt)) {
$field = sqlsrv_get_field($stmt, 0, SQLSRV_PHPTYPE_STREAM(SQLSRV_ENC_CHAR));
Copy link
Contributor

@yitam yitam Jan 30, 2017

Choose a reason for hiding this comment

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

Is Unicode is used, would SQLSRV_ENC_CHAR work?


// Compare output
$row = sqlsrv_fetch_array($stmt);
echo ($row['c1'] == $data) ? "True\n" : "False\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

No trimming necessary? Nice. :) Just curious, have you tried using '===' for comparison?


// Compare output
$row = sqlsrv_fetch_array($stmt);
echo ($row['c1'] == $data) ? "True\n" : "False\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

The main diff of this test comparing to the one above is UTF-8 charset right? Are both files saved in the same encoding?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 37.892% when pulling 97f44e3 on v-kigos:PHP-7.0-Linux into 78598fc on Microsoft:PHP-7.0-Linux.

@coveralls
Copy link

coveralls commented Jan 30, 2017

Coverage Status

Coverage increased (+0.7%) to 37.892% when pulling 97f44e3 on v-kigos:PHP-7.0-Linux into 78598fc on Microsoft:PHP-7.0-Linux.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 37.892% when pulling 50aa191 on v-kigos:PHP-7.0-Linux into 78598fc on Microsoft:PHP-7.0-Linux.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.1%) to 38.267% when pulling d74537d on v-kigos:PHP-7.0-Linux into 78598fc on Microsoft:PHP-7.0-Linux.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.1%) to 38.267% when pulling d74537d on v-kigos:PHP-7.0-Linux into 78598fc on Microsoft:PHP-7.0-Linux.

@v-dareck v-dareck merged commit bdcbc7e into microsoft:PHP-7.0-Linux Jan 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants