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

adjust sql_data_type and column_size for NULL parameters #1311

Merged
merged 5 commits into from
Oct 20, 2021

Conversation

gjcarrette
Copy link
Contributor

This is a proposed fix for #1310

@codecov
Copy link

codecov bot commented Oct 5, 2021

@yitam
Copy link
Contributor

yitam commented Oct 6, 2021

@gjcarrette

Please make changes to the method sqlsrv_param::process_null_param() instead (line 2146), because that is for input parameters:

--- a/source/shared/core_stmt.cpp
+++ b/source/shared/core_stmt.cpp
@@ -2151,13 +2151,13 @@ void sqlsrv_param::process_null_param(_Inout_ zval* param_z)
         // if the server type is a binary type, than the server expects the sql_type to be binary type
         // as well, otherwise an error stating "Implicit conversion not allowed.." is thrown by the
         // server. For all other server types, setting the sql_type to sql_char works fine.
-        sql_data_type = (encoding == SQLSRV_ENCODING_BINARY) ? SQL_BINARY : SQL_CHAR;
+        sql_data_type = (encoding == SQLSRV_ENCODING_BINARY) ? SQL_BINARY : SQL_VARCHAR;
     }

     c_data_type = (encoding == SQLSRV_ENCODING_BINARY) ? SQL_C_BINARY : SQL_C_CHAR;

     if (column_size == SQLSRV_UNKNOWN_SIZE) {
-        column_size = 1;
+        column_size = (encoding == SQLSRV_ENCODING_BINARY) ? 1 : 0;
         decimal_digits = 0;
     }
     buffer = NULL;

@yitam
Copy link
Contributor

yitam commented Oct 19, 2021

@gjcarrette any update? Would you want me to make the suggested changes?

@gjcarrette
Copy link
Contributor Author

Yes, that makes sense, because the SQLRelay source file odbc.cpp that I mentioned in the issue also uses SQL_C_BINARY for both binary or non-binary T-SQL NULL.

So one thing we didn't do here is make a phpt file, a test case, that fails with the old code but passes with the new code.
To tell the truth I did not use the make test targets in the msphpsql tree, I used the test file that I distributed with the issue.

@yitam
Copy link
Contributor

yitam commented Oct 20, 2021

Thanks @gjcarrette this looks good.

@yitam yitam merged commit 8de0978 into microsoft:dev Oct 20, 2021
@gjcarrette gjcarrette deleted the gcarrette_fix_isnull branch October 20, 2021 17:25
@lelia
Copy link

lelia commented Oct 22, 2021

Hello @yitam, I'm reaching out from Wayfair's Open Source Program Office. We're holding a company-wide Hacktoberfest event, and were hoping to recognize @gjcarrette's contribution here as part of the festivities.

In order to receive full credit for Hacktoberfest, accepted PRs must include a hacktoberfest-accepted label. Is there any chance you could apply that label to this PR so we can more easily highlight this contribution? Thanks so much.

@yitam
Copy link
Contributor

yitam commented Oct 25, 2021

Done @lelia

@lelia
Copy link

lelia commented Oct 25, 2021

Fantastic. Thanks again @yitam !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants