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

sqlsrv extension modifies the locale settings and possibly breaks other software #1063

Closed
marvinhinz opened this issue Dec 3, 2019 · 18 comments

Comments

@marvinhinz
Copy link

+## PHP Driver version or file name
ExtensionVer => 5.6.1
+## SQL Server version
not relevant
+## Client operating system
Debian 10
+## PHP version
7.1.33
+## Microsoft ODBC Driver version
not relevant
+## Table schema
not relevant
+## Problem description
The PHP Extension seems to override the LC_ options. This triggered a Problem in Magento 1 Shops. All prices were printed way to high. But this is not a fault of magento. I dont think a PHP extension should modify the locale settings at all.

Testing the output of localeconv with sqlsrv enabled:

/opt/php-7.1/bin/php  -r "print_r(localeconv());"
Array
(
    [decimal_point] => ,
    [thousands_sep] => .
    [int_curr_symbol] => EUR
    [currency_symbol] => €
    [mon_decimal_point] => ,
    [mon_thousands_sep] => .
    [positive_sign] =>
    [negative_sign] => -
    [int_frac_digits] => 2
    [frac_digits] => 2
    [p_cs_precedes] => 0
    [p_sep_by_space] => 1
    [n_cs_precedes] => 0
    [n_sep_by_space] => 1
    [p_sign_posn] => 1
    [n_sign_posn] => 1
    [grouping] => Array
        (
            [0] => 3
            [1] => 3
        )

    [mon_grouping] => Array
        (
            [0] => 3
            [1] => 3
        )

)

Testing the output of localeconv with sqlsrv disabled :

/opt/php-7.1/bin/php  -r "print_r(localeconv());"
Array
(
    [decimal_point] => .
    [thousands_sep] =>
    [int_curr_symbol] =>
    [currency_symbol] =>
    [mon_decimal_point] =>
    [mon_thousands_sep] =>
    [positive_sign] =>
    [negative_sign] =>
    [int_frac_digits] => 127
    [frac_digits] => 127
    [p_cs_precedes] => 127
    [p_sep_by_space] => 127
    [n_cs_precedes] => 127
    [n_sep_by_space] => 127
    [p_sign_posn] => 127
    [n_sign_posn] => 127
    [grouping] => Array
        (
        )

    [mon_grouping] => Array
        (
        )

)

+## Expected behavior and actual behavior

  • The extension should not modify locale config
@yitam
Copy link
Contributor

yitam commented Dec 3, 2019

Hi @marvinhinz what's the output when you run locale? What are your default LC_ options?

For example, in my env, this is what I have:

root@b5112ea0f810:/tmp# locale
LANG=en_US.UTF-8
LANGUAGE=en_US:en
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_PAPER="en_US.UTF-8"
LC_NAME="en_US.UTF-8"
LC_ADDRESS="en_US.UTF-8"
LC_TELEPHONE="en_US.UTF-8"
LC_MEASUREMENT="en_US.UTF-8"
LC_IDENTIFICATION="en_US.UTF-8"
LC_ALL=en_US.UTF-8
root@b5112ea0f810:/tmp# php --ri sqlsrv

sqlsrv

sqlsrv support => enabled
ExtensionVer => 5.6.1

Directive => Local Value => Master Value
sqlsrv.WarningsReturnAsErrors => On => On
sqlsrv.LogSeverity => 0 => 0
sqlsrv.LogSubsystems => 0 => 0
sqlsrv.ClientBufferMaxKBSize => 10240 => 10240
root@b5112ea0f810:/tmp# php  -r "print_r(localeconv());"
Array
(
    [decimal_point] => .
    [thousands_sep] => ,
    [int_curr_symbol] => USD 
    [currency_symbol] => $
    [mon_decimal_point] => .
    [mon_thousands_sep] => ,
    [positive_sign] => 
    [negative_sign] => -
    [int_frac_digits] => 2
    [frac_digits] => 2
    [p_cs_precedes] => 1
    [p_sep_by_space] => 0
    [n_cs_precedes] => 1
    [n_sep_by_space] => 0
    [p_sign_posn] => 1
    [n_sign_posn] => 1
    [grouping] => Array
        (
            [0] => 3
            [1] => 3
        )

    [mon_grouping] => Array
        (
            [0] => 3
            [1] => 3
        )

)
root@b5112ea0f810:/tmp# 

@marvinhinz
Copy link
Author

$ locale

LANG=de_DE.UTF-8
LANGUAGE=
LC_CTYPE="de_DE.UTF-8"
LC_NUMERIC="de_DE.UTF-8"
LC_TIME="de_DE.UTF-8"
LC_COLLATE="de_DE.UTF-8"
LC_MONETARY="de_DE.UTF-8"
LC_MESSAGES="de_DE.UTF-8"
LC_PAPER="de_DE.UTF-8"
LC_NAME="de_DE.UTF-8"
LC_ADDRESS="de_DE.UTF-8"
LC_TELEPHONE="de_DE.UTF-8"
LC_MEASUREMENT="de_DE.UTF-8"
LC_IDENTIFICATION="de_DE.UTF-8"
LC_ALL=

$ /opt/php-7.1/bin/php --ri sqlsrv

sqlsrv

sqlsrv support => enabled
ExtensionVer => 5.6.1

Directive => Local Value => Master Value
sqlsrv.WarningsReturnAsErrors => On => On
sqlsrv.LogSeverity => 0 => 0
sqlsrv.LogSubsystems => 0 => 0
sqlsrv.ClientBufferMaxKBSize => 10240 => 10240

@yitam
Copy link
Contributor

yitam commented Dec 4, 2019

HI @marvinhinz

It's because the driver currently sets the application locale to the system one, which in your case is de_DE.UTF-8. In other words, the driver essentially does this:

php -r "setlocale(LC_ALL, ''); print_r(localeconv());"

@marvinhinz
Copy link
Author

Hi @yitam

Okay this is strange, to workaround our price bug, we had to add setlocale(LC_ALL,"C"); in magento core files.

@stollr
Copy link

stollr commented Dec 5, 2019

It's because the driver currently sets the application locale to the system one, which in your case is de_DE.UTF-8. In other words, the driver essentially does this:

php -r "setlocale(LC_ALL, ''); print_r(localeconv());"

@yitam but this behaviour has some very unexpected and hard to debug side effects, which breaks other code. This should not happen.

As you know, setlocale(LC_ALL, 0) can be used to retrieve the current locale settings. With sqlserv extension loaded, we get this output:

$ php -r "var_dump(setlocale(LC_ALL, 0));"
string(11) "de_DE.UTF-8"

Without the extension the output looks quite different:

$ php -r "var_dump(setlocale(LC_ALL, 0));"
string(170) "LC_CTYPE=de_DE.UTF-8;LC_NUMERIC=C;LC_TIME=C;LC_COLLATE=C;LC_MONETARY=C;LC_MESSAGES=C;LC_PAPER=C;LC_NAME=C;LC_ADDRESS=C;LC_TELEPHONE=C;LC_MEASUREMENT=C;LC_IDENTIFICATION=C"

And as @marvinhinz has already pointed out, Magento (version 1) that uses the Zend Framework's locale component (see https://framework.zend.com/manual/1.12/en/zend.locale.functions.html) struggels with the setting done by sqlsrv extension.

Can you tell me, why the extension changes the locale settings?

@yitam
Copy link
Contributor

yitam commented Dec 5, 2019

Hi @marvinhinz, glad to hear you found a workaround.

Hi @Naitsirch, please be reminded that when calling setlocale(LC_ALL, 0), if not all categories have the same values then a string containing all the locales will be returned. That explains why you see a long string of length 170, because some have the default C locale.

To prove my point, try the following:

$ export LANG=C

$ php -r "var_dump(setlocale(LC_ALL, 0));"
string(1) "C"

Or you can try something like this (with sqlsrv disabled):

$ export LANG='en_US.UTF-8'

$ php -r "var_dump(setlocale(LC_ALL, 0));"
string(170) "LC_CTYPE=en_US.UTF-8;LC_NUMERIC=C;LC_TIME=C;LC_COLLATE=C;LC_MONETARY=C;LC_MESSAGES=C;LC_PAPER=C;LC_NAME=C;LC_ADDRESS=C;LC_TELEPHONE=C;LC_MEASUREMENT=C;LC_IDENTIFICATION=C"

$ php -r "setlocale(LC_ALL, 'en_US.UTF-8'); var_dump(setlocale(LC_ALL, 0));"
string(11) "en_US.UTF-8"

$ php -r "setlocale(LC_ALL, 'en_US.UTF-8'); var_dump(setlocale(LC_MESSAGES, 0));"
string(11) "en_US.UTF-8"

$ php -r "setlocale(LC_ALL, 'en_US.UTF-8'); var_dump(setlocale(LC_NUMERIC, 0));"
string(11) "en_US.UTF-8"

We had users in the past who wanted sqlsrv or pdo_sqlsrv drivers to work with their non-UTF-8 locale(s) in their Linux env. Hence, the values of their locale environment variables are used.

That being said, we may consider providing a configurable option for users. I can mark this issue as a feature request if you like.

@stollr
Copy link

stollr commented Dec 5, 2019

Thanks for the explanation. An option to define this behavior would be the best solution I think. The question is, what is the default behavior?

IMHO the activation of an extension should not change the behavior of other parts of the language or library per default. If the user definitely want that and defines that via config file or code, it's okay.

Another question: Can you tell me where I can find the code in the sqlsrv extension that changes the locale setting? I tried to find it but wasn't successful.

@yitam
Copy link
Contributor

yitam commented Dec 5, 2019

hi @Naitsirch , you can find it here

We will discuss how / whether to make this configurable, but in order not to introduce a breaking change the default might likely be the way it is now.

@stollr
Copy link

stollr commented Dec 5, 2019

@yitam Thanks for pointing me to the right place :-)

I do understand your BC concerns. Maybe an idea to achieve my proposed idea with developer and administrator friendly upgrade path:

  • Introduce configuration in next 5.x release and trigger a deprecation message if the configuration is not set. But keep the default as it is now. This will make sure that everyone set the config the way they need it.
  • Then in 6.0 the default behavior can be changed to not changing locale settings without explicit config setting.

That's the way BC breaks are handled in projects like Symfony or PHP core.

@yitam
Copy link
Contributor

yitam commented Dec 11, 2019

Thanks @Naitsirch. We will keep your suggestions in mind when we decide our next course of action.

david-puglielli added a commit that referenced this issue Jan 31, 2020
* Fixed the potential error reported by Prefast code analysis

* Use SQLSRV_ASSERT for checking NULL ptrs

* For these AKV tests check env despite not AE connected

* Added the driver option to run functional tests

* Fixed connection pooling tests for more than one ODBC drivers

* added driver option to pdo isPooled.php

* Removed win32 ifdefs re connection resiliency (#802)

* Set the driver argument for getDSN to null by default (#798)

* Added the driver argument to getDSN

* Dropped the driver argument but set to null as default

* Removed the AE condition in locale support

* Modified the AE condition for locale support

* Changed int to SQLLEN to avoid infinite loop (#806)

* Version 5.3.0 (#803)

* Version 5.3.0

* Fixed the wrong replacements

* Added comments block to m4 files

* Use dnl for comments

*  Modified AE fetch phptypes test to insert only one row at a time and loop through php types (#801)

* Modified AE fetch phptypes test to insert only one row at a time and loop through php types

* Fixed formatting

* Streamlined two very similar large column name tests (#807)

* Streamlined two very similar large column name tests

* Changed the EOL

* Updates to change log and readme (#811)

* Updates to change log and readme

* Dropped support for Ubuntu 17

* Modified as per review comments

* Fixed connection resiliency tests for Unix, updated AppVeyor for ODBC 17.2

* Fixed expected output

* Fixed output and skipifs

* Fixed skipifs and output

* Fixed driver name

* Updated installation instructions and sample script (#813)

* Updated instructions and sample test for 5.3.0 RTW

* Fixed sample code to adhere to php coding standard

* Fixed cases and spaces

* Modified NOTE for UB 18.04 based on review comments

* Added 'exit'

* Modified change log and readme based on review to PR 811

* Applied review comments

* build output to debug appveyor failure

* removed debug output

* Streamlined two very similar large column name tests (#815)

* Streamlined two very similar large column name tests

* Added random number of test table names to avoid operand clash issues

* Replaced to with for based on review

* Changelog updated

* changelog updated, test skipif changed to run on unix platforms

* Fixed skipif typo

* Fixed typo in skipif for pdo

* Fixed some output for Travis

* Moved error checking inside pdo connres tests

* Added links back to changelog

* Fixed output for sqlsrv connres tests

* Fixed output

* Fixed output again

* Fixed skipifs for connres

* Tweaked per review comments

* Changes made to source and tests to support PHP 7.3 (#822)

* Changes made to support php 7.3

* Correct use of the smart pointer

* Fixed the tests for 7.3

* Some clean up for array_init()

* Fixed formattings and clean up

* One more fix

* Initialising strings with nulls

* Removed some spaces

* Made array index spacing consistent

* Fix for compilation problem

* Fix for compilation problem again

* Before freeing stmt in destructor check if dbh driver data is NULL  (#829)

* Issue 434 - set dbh driver data to NULL as well in destructor

* Reverted the last change but instead check if dbh driver_data is already freed

* Modified the comment

* Added driver to the skipif conditions (#831)

* Used git clone instead to download source from a branch of a tag (#832)

* Modified the error handling to make it more flexible (#833)

* Made error handling more flexible

* Fixed a minor issue with a test

* Enabled Spectre Mitigations (#836)

* Incorporated changes in PR 634 to pdo_sqlsrv (#834)

* Incorporated changes in PR 634 to pdo_sqlsrv

* Reverted the changes because the array is for internal use only

* Modified README re user's suggestion (#841)

* Modified README re user's suggestion

* Moved the if condition to the end as per review

* Adding supporting for Azure AD access token (#837)

* Adding supporting for Azure AD access token

* Added more comments for the AD access token skipif files

* Save the pointer to access token struct until after connecting

* Clear the access token data before freeing the memory

* Added a reference as per review

* Feature request - new PDO_STMT_OPTION_FETCHES_DATETIME_TYPE flag for pdo_sqlsrv to return datetime as objects (#842)

* Feature request - issue 648

* Fixed constructor for field_cache and added another test

* Added tests for FETCH_BOUND

* Added a new test for output param

* Modified output param test to set attributes differently

* Removed a useless helped function in a test

* Combined two new tests into one as per review

* Uncommented dropTable

* Feature request - add ReturnDatesAsStrings option to statement level for sqlsrv  (#844)

* Added ReturnDatesAsStrings option to the statement level

* Added new tests for ReturnDatesAsStrings at statement level

* Added more datetime types as per review

* Updated version 5.4.0-preview (#846)

* Updated version 5.4.0-preview

* Replaced 5.3 with 5.4

* Fixed sqlsrv datetime tests to connect with ColumnEncryption variables (#849)

* Change log for 5.4.0-preview (#850)

* Updated change log for 5.4.0-preview

* Updated 5.4.0 preview to add two new feature requests

* Modified change log as per review

* Modified the wordings

* Updated readme, changelog, and install instructions

* Clear AKV data after setting the connection attribute or when exception is thrown (#854)

* Dev (#820)

* Fixed the potential error reported by Prefast code analysis

* Use SQLSRV_ASSERT for checking NULL ptrs

* For these AKV tests check env despite not AE connected

* Added the driver option to run functional tests

* Fixed connection pooling tests for more than one ODBC drivers

* added driver option to pdo isPooled.php

* Removed win32 ifdefs re connection resiliency (#802)

* Set the driver argument for getDSN to null by default (#798)

* Added the driver argument to getDSN

* Dropped the driver argument but set to null as default

* Removed the AE condition in locale support

* Modified the AE condition for locale support

* Changed int to SQLLEN to avoid infinite loop (#806)

* Version 5.3.0 (#803)

* Version 5.3.0

* Fixed the wrong replacements

* Added comments block to m4 files

* Use dnl for comments

*  Modified AE fetch phptypes test to insert only one row at a time and loop through php types (#801)

* Modified AE fetch phptypes test to insert only one row at a time and loop through php types

* Fixed formatting

* Streamlined two very similar large column name tests (#807)

* Streamlined two very similar large column name tests

* Changed the EOL

* Updates to change log and readme (#811)

* Updates to change log and readme

* Dropped support for Ubuntu 17

* Modified as per review comments

* Fixed connection resiliency tests for Unix, updated AppVeyor for ODBC 17.2

* Fixed expected output

* Fixed output and skipifs

* Fixed skipifs and output

* Fixed driver name

* Updated installation instructions and sample script (#813)

* Updated instructions and sample test for 5.3.0 RTW

* Fixed sample code to adhere to php coding standard

* Fixed cases and spaces

* Modified NOTE for UB 18.04 based on review comments

* Added 'exit'

* Modified change log and readme based on review to PR 811

* Applied review comments

* build output to debug appveyor failure

* removed debug output

* Streamlined two very similar large column name tests (#815)

* Streamlined two very similar large column name tests

* Added random number of test table names to avoid operand clash issues

* Replaced to with for based on review

* Changelog updated

* changelog updated, test skipif changed to run on unix platforms

* Fixed skipif typo

* Fixed typo in skipif for pdo

* Fixed some output for Travis

* Moved error checking inside pdo connres tests

* Added links back to changelog

* Fixed output for sqlsrv connres tests

* Fixed output

* Fixed output again

* Clear AKV data after connection or when exception is thrown

* Modified tests too to skip some AKV tests without real credentials

* Used assignment operator also free the existing memory

* Change readme links to https

* Change readme links to https

Merging this commit to dev

* Save meta data for the fetched result set (#855)

* Save meta data on fetched result sets

* Fixed a compilation error

* Optimized some more -- metadata should be available when fetching

* Skip conversion for strings of numeric values, integers, floats, decimals etc

* Set encoding char for numeric data

* Apply review

* Added Mojave to macOS instructions (#862)

Added Mojave to macOS instructions

* Fixed the broken links of Appveyor status badge (#863)

* Feature request 415 for sqlsrv (#861)

* Modified how to send stream data using SQLPutData and SQLParamData (#865)

* Updated instructions to include Ubuntu 18.10 (#869)

* Feature request 415 for pdo_sqlsrv (#873)

* Skipped some tests when running against Azure (#874)

* Modified config files to add the compiler flag, /Qspectre (#878)

* Merge the commit from master re survey image link (#880)

* Fixed the flaws of decimal tests and added more debugging (#879)

* Changed sample code to adhere to PSR standard (#887)

* Decimal places for money types only (#886)

* Version update for 5.5.0-preview (#889)

* Fixed the error in the pdo decimal test (#890)

* Removed warning messages while compiling extensions (#892)

* Improve performance of Unicode conversions (#891)

* Update sqlsrv_statement_format_money_scales.phpt

Do not encrypt money / smallmoney fields in the test table

* Change log 5.5.0-preview (#895)

* updated docs for php 7.3

* Fixed broken links

* Added back Ubuntu 18.10 ODBC instruction

* Drop tests related to fake passwords (#905)

* Initialize output param buffer when allocating extra space (#907)

* Enable compiling extensions statically into PHP (#904)

* Dropped dbname variable and set QUOTED_IDENTIFIER to ON (#911)

* Skipped the non-applicables tests against Azure Data Warehouse (#913)

* Support for Managed Identity for Azure resources (#875)

* Changed version 5.6.0 (#918)

* Initialize hasLoss before passing into Convert function (#919)

* Added new tests for setting client buffer size related to issue 228 (#920)

* Fixed load order issue in sqlsrv

* Added source indexing for symbols (#922)

* Modified linux and mac instructions for 5.6.0 RTW (#926)

* Change log 5.6.0 (#921)

* add Language option on connect

* Updated AppVeyor to download ODBC driver 17.3 (#941)

* Issue 937 - fixed ASSERT and added new tests (#940)

* Changed travis to pull mcr.microsoft.com/mssql/server:2017-latest instead (#943)

* Modified money tests to test the accuracies of floats (#944)

* Fixed the returned values for PDOStatement::getColumnMeta (#946)

* Onboarding to Azure Pipelines (#949)

* Fixed the error in Issue 570  (#952)

* Added a new status badge on readme (#953)

* Added new tests for issue 569 (#951)

* Fix issue 955 - errors building sqlsrv alone (#956)

* Modified test_largeData for Linux CI (#954)

* Issue 937 - fixed ASSERT and added new tests (#940)


(cherry picked from commit 12d01c9)

* Fixed the returned values for PDOStatement::getColumnMeta (#946)


(cherry picked from commit 7309fb9)

* Fix issue 955 - errors building sqlsrv alone (#956)

(cherry picked from commit 15f61bd)

* 5.6.1 hotfix

* Updated change log

* Tests modified for language option for SQL Azure (#963)

* Update azure-pipelines.yml for Azure Pipelines [skip ci] (#964)

* Added more checks for error conditions (#965)

* Removed forward cursor condition

* Added row and column count checks

* Revert "Update azure-pipelines.yml for Azure Pipelines [skip ci] (#964)" (#969)

This reverts commit 7d389e0.

* Add new pdo_sqlsrv tests for utf8 encoding errors (#966)

* Modified to check if qualified for AE connections (#967)

* Fixed test and error message

* Minor fixes

* Test fixes

* Addressed review comments

* Fixed test failure

* Made Azure AD tests more robust (#973)

* Addressed review comments

* Issue 970: use quotes for variables (#971)

* Added batch query test

* Fixed 32 bit test failure

* Addressed review comments

* Formatting changes

* Used different skipif conditions for these two tests that require AE connections (#977)

* Simplified insert logic

* Modified get column meta method to reference saved metadata (#978)

* Revert "Used different skipif conditions for these two tests that require AE connections (#977)" (#980)

This reverts commit ee3c85a.

* Fixed failing tests (#981)

*  Data Classification sensitivity metadata retrieval (#979)

* Added more pdo tests to verify different error conditions (#984)

* Fixed memory issues with data classification (#985)

* Added connection string flag

* Removed unix skipif

* Fixed test output

* Fixed pdo test

* Changed flag name

* Fixed test output

* Updated links and versions (#987) (#988)

* Fixed test output (again)

* Fixed test output (again)

* Fixed test output (again)

* Replaced expected test output altogether

* Fixed locale issue

* Corrected formatting

* Replaced EXPECTF with EXPECT

* Fixed two failing tests (#991)

* Redesigned some tests based on recent test results (#992)

* Modified pipelines to connect using sqlcmd inside of the container instead (#995)

* Added batch query

* Added batch query test for pdo (#997)

* Added a new test and modify a non LOB sqlsrv test (#1000)

* Two index zval functions are macros in php 7.4 (#1001)

* Replaced uint with size_t (#1004)

*  Check compiler version for php 74 (#1005)

* Fixed tests that failed in php 7.4 (#1006)

* Improve data caching with datetime objects (#1008)

* Fixed for issues found by Semmle (#1011)

* Removed unneeded constants

* Fixed sqlsrv_free_stmt argument info

* Fixed brace escape to avoid buffer overflow

* Fixed brace escape and added test

* Debugging test failure on Bamboo

* Removed debugging output

* Debugging test failure on Bamboo

* Removed debugging output

* Added more test cases

* Changed range check to use strchr

* Added pdo test

* Fixed test and formatting

* Addressed various issues with PHP 7.4 beta1 (#1015)

* Updated dockerfile to use UB 18.04 and PHP 73 (#1016)

* Added survey results (#1017)

* Updated ODBC driver 17.4 (#1019)

* Modified output.py to take a new argument and travis yml to use include for coveralls (#1020)

*  Used constants in memory stress tests for easier configuration (#1022)

* Removed KSP related scripts and files (#1030)

* Updated version to 5.7.0 preview (#1029)

* Change log for 5.7.0 (#1028)

* Modified how drivers handle query timeout settings (#1037)

* Feature request: support extended string types (#1043)

* Added the required file to ansi tests (#1047)

* Always Encrypted v2 support (#1045)

* Change to support ae-v2

* Add support for AE V2

* Added some descriptions and comments

* Fixed PDO pattern matching

* Updated key generation scripts

* Fixed key script

* Fixed char/nchar results, fixed formatting issues

* Addressed review comments

* Updated key scripts

* Debugging aev2 keyword failure

* Debugging aev2 keyword failure

* Debugging aev2 keyword failure

* Debugging aev2 keyword failure

* Added skipif to ae v2 keyword test

* Addressed review comments

* Fixed braces and camel caps

* Updated test descriptions

* Added detail to test descriptions

* Tiny change

* Modified pdo tests to work with column encryption (#1051)

* Saved php types with metadata when fetching (#1049)

* Updated survey charts for Nov 2019 (#1057)

* Updated all CIs (#1058)

* Change log 5.7.1 preview (#1060)

* Fix AKV keyword test for AE v2 behaviour (#1061)

* Master (#936)

5.6.0 RTW

* 5.6.1 hotfix (#959)

* Updated links and versions (#987)

* Fixed AKV keyword tests for AE v2

* Added comment

* Free proc cache before starting test

* Fixed comment

* Update linux mac instructions for php 7.4 (#1062)

* Updated appveyor yml to build 7.3 and 7.4 (#1065)

* Fixes suggested by Semmle (#1068)

* Fixes suggested by Semmle

* Updated azure-pipelines

* Added configurable options for setting locales (#1069)

#1063

* Fixed the skipif wordings and styles (#1070)

* Modified locale tests to work in both linux and mac (#1074)

* Include sql_variant type for buffered queries (#1080)

* Updated versions and year (#1082)

* Change log for version 5.8.0 (#1083)

* 5.8.0 rtw docs (#1086)

* updated install instructions and changelog

* removed md extensions

* Addressed review comments

* added path

* Fixed link

Co-authored-by: Jenny Tam <[email protected]>
Co-authored-by: Gert de Pagter <[email protected]>
Co-authored-by: Jannes Jeising <[email protected]>
Co-authored-by: Guillaume Degoulet <[email protected]>
@yitam
Copy link
Contributor

yitam commented Feb 3, 2020

Hi @marvinhinz @Naitsirch

We just released 5.8.0 and please check the latest documentation concerning this issue

@stollr
Copy link

stollr commented Feb 3, 2020

Looks good! Thank you

@yitam
Copy link
Contributor

yitam commented Feb 3, 2020

Great! Thanks for the confirmation @Naitsirch. Closing this issue now. Please feel free to reopen if necessary.

@yitam yitam closed this as completed Feb 3, 2020
@grossmannmartin
Copy link

In my opinion, this issue is not solved.
Consider the following scenario.
I have my developer machine set to my national locale (diacritics, etc.), PHP application sets it's own locale settings independently on host machine. After installing the extension, application is broken due to forced locale settings, and the only way how to fix it is to reconfigure the system (LC_ variables are used by other applications too).

I suggest adding a directive to set specific locale or remove this magic setting inside completely.

@yitam
Copy link
Contributor

yitam commented May 26, 2020

@grossmannmartin I don't quite understand the issue. Please elaborate.

Did you add the driver setting in the php.ini as explained in the documentation?

@grossmannmartin
Copy link

@yitam Sorry, I try to describe the issue better.

  1. My notebook is set to Czech locale. I do not have set LC_* variables.

  2. Bootstrap of my application contains following

setlocale(LC_CTYPE, 'en_US.utf8');
setlocale(LC_NUMERIC, 'en_US.utf8');

var_dump(localeconv());

output is

array:18 [
  "decimal_point" => "." 
  "thousands_sep" => ""
  "int_curr_symbol" => ""
  "currency_symbol" => ""
...
  1. I enable sqlsrv extension

  2. output of the code above is

array:18 [
  "decimal_point" => ","
  "thousands_sep" => " "
  "int_curr_symbol" => "CZK "
  "currency_symbol" => "Kč"
  1. I set sqlsrv.SetLocaleInfo under [sqlsrv] section to any value (0, 1, 2)
  • no effect
  1. I have to set environment variable LC_CTYPE to en_US.utf8
  • now the output is as it should be

But any other software depending on this variable break Czech language (git for example)
I have to alter my system settings to make the extension work properly even when I set the locale explicitly in the application.

@yitam
Copy link
Contributor

yitam commented May 27, 2020

@grossmannmartin did you happen to have installed pdo_sqlsrv as well? If so, you will need to set locale info for pdo_sqlsrv to 0. If not, please create a new issue and reference this one and we will help you from there.

@grossmannmartin
Copy link

@yitam Thank you :) it was really a problem related to pdo_sqlsrv.
After setting pdo_sqlsrv.set_locale_info = 0 in my php.ini file everything works like a charm.

Sorry for the fuss. You rock 👍

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

No branches or pull requests

4 participants