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

add Language option on connect #930

Merged
merged 1 commit into from
Mar 28, 2019
Merged

Conversation

gdegoulet
Copy link
Contributor

No description provided.

@msftclas
Copy link

msftclas commented Feb 13, 2019

CLA assistant check
All CLA requirements met.

@codecov-io
Copy link

codecov-io commented Feb 13, 2019

Codecov Report

Merging #930 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #930   +/-   ##
=======================================
  Coverage   80.51%   80.51%           
=======================================
  Files          25       25           
  Lines        7753     7753           
=======================================
  Hits         6242     6242           
  Misses       1511     1511
Impacted Files Coverage Δ
...x86/php-7.1.26-src/ext/sqlsrv/shared/core_stmt.cpp 85.67% <0%> (-0.48%) ⬇️
...php-7.1.26-src/ext/pdo_sqlsrv/shared/core_sqlsrv.h 83.5% <0%> (ø) ⬆️
...phpdev/vc14/x86/php-7.1.26-src/ext/sqlsrv/conn.cpp 83.88% <0%> (ø) ⬆️
...x86/php-7.1.26-src/ext/sqlsrv/shared/core_sqlsrv.h 86.86% <0%> (ø) ⬆️
...vc14/x86/php-7.1.26-src/ext/pdo_sqlsrv/pdo_dbh.cpp 92.11% <0%> (ø) ⬆️
...php-7.1.26-src/ext/pdo_sqlsrv/shared/core_stmt.cpp 75.64% <0%> (+0.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5989d8...5b2b750. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 72.431% when pulling 5b2b750 on gdegoulet:dev into c5989d8 on Microsoft:dev.

@david-puglielli
Copy link
Contributor

Thanks for this PR @gdegoulet. Please be aware of how the language functionality works:

There are three layers of messages that may be returned when using the PHP drivers. One layer comes from the PHP driver, and because the PHP driver has not been localised, those messages will always be in English. Another layer comes from the ODBC driver (and driver manager). To get messages from the ODBC driver in other languages, you need to use a localised version of the driver in the language you prefer. These localised versions are available to download from the Microsoft Download Center. The third layer is messages that come from SQL Server itself. When the Language option is passed to the ODBC driver, it affects only the messages received from SQL Server. So if the user connects with the Language option, messages may be returned in multiple languages because they could come from any of the three layers.

However, this is not much different from the current user experience with the ODBC driver, since you would still get messages in different languages if you pass a different language as a connection attribute than a localised version of the ODBC driver uses. The difference is that with your PR, messages from SQL Server can be set directly from a PHP script, but messages from the PHP driver would still be in English.

We want to gauge how useful adding a Language option is - so, considering all the info above, do you believe the Language option should be added to the PHP driver?

@gdegoulet
Copy link
Contributor Author

Hello David : for an very old website (LAMP) , we used php 5.6 with freetds driver.
Now we are working on a new release of our website on php 7.2/7.3 with the new sqlsrv driver : we encountered problem with date format on insert operation : our first workaround was to set language after connect to mssql server : something like this : bdd, 'SET LANGUAGE us_english'); ?>

you said "When the Language option is passed to the ODBC driver, it affects only the messages received from SQL Server" : not only, it seems to change the database behaviours on date string, number format, ... ?

After some search, we discovered that there is an language option (and some other) on the connect function from sqlsrv driver but not in php extension ?
You ask "why should we add this option ?" I would say "why not ?" why all useful driver options are not present on the php extension side ?

I'm not mssql server expert : it's maybe not the good way to solve our problems ? look forward to reading your tips.

Thanks

@david-puglielli
Copy link
Contributor

Ok we will merge this PR shortly, but not before the next preview release 5.6.1, which fixes a problem with the PECL packaging. That will be in the next few days most likely. Please stay tuned.

@david-puglielli david-puglielli merged commit 6325284 into microsoft:dev Mar 28, 2019
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.

5 participants