Bug #46408 [Com]: Locale number format settings can cause pg_query_params to break with numerics
Edit report at https://bugs.php.net/bug.php?id=46408edit=1
ID: 46408
Comment by: alec at smecher dot bc dot ca
Reported by:alec at smecher dot bc dot ca
Summary:Locale number format settings can cause
pg_query_params to break with numerics
Status: Wont fix
Type: Bug
Package:PostgreSQL related
Operating System: *
PHP Version:5.*, 6
Assigned To:yohgaki
Block user comment: N
Private report: N
New Comment:
I suggest this patch.
diff -u -r php-5.4.6/ext/pgsql/pgsql.c php-5.4.6-mod/ext/pgsql/pgsql.c
--- php-5.4.6/ext/pgsql/pgsql.c 2012-08-14 21:26:05.0 -0700
+++ php-5.4.6-mod/ext/pgsql/pgsql.c 2012-09-06 10:59:45.0 -0700
@@ -23,6 +23,7 @@
/* $Id$ */
#include stdlib.h
+#include locale.h
#define PHP_PGSQL_PRIVATE 1
@@ -1736,7 +1737,15 @@
} else {
zval tmp_val = **tmp;
zval_copy_ctor(tmp_val);
+
+ // PSQL requires . for radix; convert to string,
+ // avoiding problems with doubles and locales
+ // using , as a radix character instead
+ // (see https://bugs.php.net/bug.php?id=46408)
+ char *current_locale = setlocale(LC_NUMERIC,
C);
convert_to_string(tmp_val);
+ setlocale(LC_NUMERIC, current_locale);
+
if (Z_TYPE(tmp_val) != IS_STRING) {
php_error_docref(NULL TSRMLS_CC,
E_WARNING,Error converting parameter);
zval_dtor(tmp_val);
Previous Comments:
[2012-04-18 12:44:31] claude dot pache at gmail dot com
@yohgaki (and others)
I think that, the root of the problem is the way PHP uses
the locale information, which I consider deeply broken.
Here are the details:
In my understanding, the locale information is useful only
for *output*, i.e. for messages destined to the user.
They should not be used for any internal conversion
from one type to the other, unless the result is
destined to output.
The problem is, that PHP uses the locale for any
automatic conversion from number to string.
This behaviour is ok in the following case:
echo Three and a half is: . $number;
However, in the following cases, this is NOT correct,
because the resulting string must not be localised:
* constructing a JSON object (I hope that json_encode()
does NOT use internal number-to-string conversion);
* using bcmath package (I have personnaly be bitten by
this misfeature);
* construct a SQL request (the present case);
* etc.
In all these cases, you have to do one of the following options:
(1) never use any locale other than en_US
(and re-implement manually the locale feature);
(2) carefully check the type of each and every parameter
and explicitely perform a correct conversion when needed,
e.g. using number_format(..., '.', '');
(3) fix PHP to NOT use locale for number-to-string conversion
unless it is explicitely asked for
(side note: historically, there has been a similar
problem with the magic quote misfeature);
(4) modify the modules bcmath, postgresql, etc,
so that they circumvent the mentionned PHP misfeature,
i.e., they do the option (2) above at your place.
In my dreams, the option (3) would be implemented,
but pragmatically, I think that option (4)
has more chance to be implemented rapidly, if ever.
I think that alec asked precisely the option (4) to be implemented.
(Personnally, I have opted for option (1).)
Claude
P.S. The option (4) might seem a non-optimal hack.
However, do not forget that programming languages
and API should be adapted to the needs of the programmers,
and not the other way round.
[2012-04-18 03:13:12] yohg...@php.net
You misunderstand how libpq/PostgreSQL works.
If you think you can make proper patch for this, clone git source and send pull
request.
No one will stop you from that.
[2012-04-18 02:58:26] alec at smecher dot bc dot ca
I believe pg_query_params is broken until this is resolved, but it looks like
we're not going to agree on it. I hope someone else can speak up if they do
think this is a bug.
Since we disagree on the approach any patch I write to correct it will be
rejected.
I'll add a comment to the manual page for pg_query_params to document this.
[2012-04-18 02:26:48] yohg...@php.net
BTW, you are reading
Bug #46408 [Com]: Locale number format settings can cause pg_query_params to break with numerics
Edit report at https://bugs.php.net/bug.php?id=46408edit=1
ID: 46408
Comment by: alec at smecher dot bc dot ca
Reported by:alec at smecher dot bc dot ca
Summary:Locale number format settings can cause
pg_query_params to break with numerics
Status: Wont fix
Type: Bug
Package:PostgreSQL related
Operating System: *
PHP Version:5.*, 6
Assigned To:yohgaki
Block user comment: N
Private report: N
New Comment:
Pull request filed at https://github.com/php/php-src/pull/186
Previous Comments:
[2012-09-06 18:03:28] alec at smecher dot bc dot ca
I suggest this patch.
diff -u -r php-5.4.6/ext/pgsql/pgsql.c php-5.4.6-mod/ext/pgsql/pgsql.c
--- php-5.4.6/ext/pgsql/pgsql.c 2012-08-14 21:26:05.0 -0700
+++ php-5.4.6-mod/ext/pgsql/pgsql.c 2012-09-06 10:59:45.0 -0700
@@ -23,6 +23,7 @@
/* $Id$ */
#include stdlib.h
+#include locale.h
#define PHP_PGSQL_PRIVATE 1
@@ -1736,7 +1737,15 @@
} else {
zval tmp_val = **tmp;
zval_copy_ctor(tmp_val);
+
+ // PSQL requires . for radix; convert to string,
+ // avoiding problems with doubles and locales
+ // using , as a radix character instead
+ // (see https://bugs.php.net/bug.php?id=46408)
+ char *current_locale = setlocale(LC_NUMERIC,
C);
convert_to_string(tmp_val);
+ setlocale(LC_NUMERIC, current_locale);
+
if (Z_TYPE(tmp_val) != IS_STRING) {
php_error_docref(NULL TSRMLS_CC,
E_WARNING,Error converting parameter);
zval_dtor(tmp_val);
[2012-04-18 12:44:31] claude dot pache at gmail dot com
@yohgaki (and others)
I think that, the root of the problem is the way PHP uses
the locale information, which I consider deeply broken.
Here are the details:
In my understanding, the locale information is useful only
for *output*, i.e. for messages destined to the user.
They should not be used for any internal conversion
from one type to the other, unless the result is
destined to output.
The problem is, that PHP uses the locale for any
automatic conversion from number to string.
This behaviour is ok in the following case:
echo Three and a half is: . $number;
However, in the following cases, this is NOT correct,
because the resulting string must not be localised:
* constructing a JSON object (I hope that json_encode()
does NOT use internal number-to-string conversion);
* using bcmath package (I have personnaly be bitten by
this misfeature);
* construct a SQL request (the present case);
* etc.
In all these cases, you have to do one of the following options:
(1) never use any locale other than en_US
(and re-implement manually the locale feature);
(2) carefully check the type of each and every parameter
and explicitely perform a correct conversion when needed,
e.g. using number_format(..., '.', '');
(3) fix PHP to NOT use locale for number-to-string conversion
unless it is explicitely asked for
(side note: historically, there has been a similar
problem with the magic quote misfeature);
(4) modify the modules bcmath, postgresql, etc,
so that they circumvent the mentionned PHP misfeature,
i.e., they do the option (2) above at your place.
In my dreams, the option (3) would be implemented,
but pragmatically, I think that option (4)
has more chance to be implemented rapidly, if ever.
I think that alec asked precisely the option (4) to be implemented.
(Personnally, I have opted for option (1).)
Claude
P.S. The option (4) might seem a non-optimal hack.
However, do not forget that programming languages
and API should be adapted to the needs of the programmers,
and not the other way round.
[2012-04-18 03:13:12] yohg...@php.net
You misunderstand how libpq/PostgreSQL works.
If you think you can make proper patch for this, clone git source and send pull
request.
No one will stop you from that.
[2012-04-18 02:58:26] alec at smecher dot bc dot ca
I believe pg_query_params is broken until this is resolved, but it looks like
we're not going to agree on it. I hope someone else can speak up if they do
think this is a bug.
Since we disagree on the approach any patch I write to correct it will be
rejected.
I'll add a comment to the
Bug #46408 [Com]: Locale number format settings can cause pg_query_params to break with numerics
Edit report at https://bugs.php.net/bug.php?id=46408edit=1 ID: 46408 Comment by: claude dot pache at gmail dot com Reported by:alec at smecher dot bc dot ca Summary:Locale number format settings can cause pg_query_params to break with numerics Status: Wont fix Type: Bug Package:PostgreSQL related Operating System: * PHP Version:5.*, 6 Assigned To:yohgaki Block user comment: N Private report: N New Comment: @yohgaki (and others) I think that, the root of the problem is the way PHP uses the locale information, which I consider deeply broken. Here are the details: In my understanding, the locale information is useful only for *output*, i.e. for messages destined to the user. They should not be used for any internal conversion from one type to the other, unless the result is destined to output. The problem is, that PHP uses the locale for any automatic conversion from number to string. This behaviour is ok in the following case: echo Three and a half is: . $number; However, in the following cases, this is NOT correct, because the resulting string must not be localised: * constructing a JSON object (I hope that json_encode() does NOT use internal number-to-string conversion); * using bcmath package (I have personnaly be bitten by this misfeature); * construct a SQL request (the present case); * etc. In all these cases, you have to do one of the following options: (1) never use any locale other than en_US (and re-implement manually the locale feature); (2) carefully check the type of each and every parameter and explicitely perform a correct conversion when needed, e.g. using number_format(..., '.', ''); (3) fix PHP to NOT use locale for number-to-string conversion unless it is explicitely asked for (side note: historically, there has been a similar problem with the magic quote misfeature); (4) modify the modules bcmath, postgresql, etc, so that they circumvent the mentionned PHP misfeature, i.e., they do the option (2) above at your place. In my dreams, the option (3) would be implemented, but pragmatically, I think that option (4) has more chance to be implemented rapidly, if ever. I think that alec asked precisely the option (4) to be implemented. (Personnally, I have opted for option (1).) Claude P.S. The option (4) might seem a non-optimal hack. However, do not forget that programming languages and API should be adapted to the needs of the programmers, and not the other way round. Previous Comments: [2012-04-18 03:13:12] yohg...@php.net You misunderstand how libpq/PostgreSQL works. If you think you can make proper patch for this, clone git source and send pull request. No one will stop you from that. [2012-04-18 02:58:26] alec at smecher dot bc dot ca I believe pg_query_params is broken until this is resolved, but it looks like we're not going to agree on it. I hope someone else can speak up if they do think this is a bug. Since we disagree on the approach any patch I write to correct it will be rejected. I'll add a comment to the manual page for pg_query_params to document this. [2012-04-18 02:26:48] yohg...@php.net BTW, you are reading PostgreSQL manual wrong. libpq's functions never care about data types, but the server is. If you are curious still, try to make patch that meets the requirement I've wrote. [2012-04-18 02:19:38] yohg...@php.net IIRC, MDB2 cares data types. (which I think it's a design problem for loosely type langs.) Therefore, it may change behavior according to locale. Anyway, It's not a matter of argument. It's the way it works. As I explained repeatedly, ALL params are passed as string and types are NEVER cared in C API. To know the data type, module should issue an additional query to get meta data. Additional query for each query is severe performance hit. Therefore, it is a PHP programmer's responsibility (or Perhaps, PostgreSQL. If they would like to change behavior via client environment vars. I guess they would not.) If you could submit patch that works for all data types and does not require additional query, I'll review it. [2012-04-18 01:30:38] alec at smecher dot bc dot ca Your example of storing a string-formatted double in a varchar column strikes me as an unusual case involving a type mismatch, and can be worked around clearly and logically by passing $number in rather than $number. If this isn't going to be fixed, there
Bug #46408 [Com]: Locale number format settings can cause pg_query_params to break with numerics
Edit report at http://bugs.php.net/bug.php?id=46408edit=1 ID: 46408 Comment by: lewis at peppermind dot de Reported by: alec at smecher dot bc dot ca Summary: Locale number format settings can cause pg_query_params to break with numerics Status: Open Type: Bug Package: PostgreSQL related Operating System: * PHP Version: 5.*, 6 New Comment: This issue also appears with pg_execute(), when passing float values to the bind array, with a locale that uses comma as decimal separator (such as de_DE and most other european locales). Previous Comments: [2009-07-26 18:59:12] jerico dot dev at gmail dot com @jani: When I pass in a double, I expect pg_query_params() to prepare it in a way that can be understood by the database independent of my locale settings. AFAIK the implementation of pg_query_params() is also inconsistent with that of the mysql driver which correctly accepts double typed parameters independent of locale. I guess you were not entirely serious when you proposed that one should switch the locale before using pg_query_params(), were you? [2008-11-21 13:09:19] j...@php.net I guess it's an issue always if extension does 'convert_to_string()'. Easily avoided in code: Only do setlocale() prior to outputting stuff. And then restore the locale right after output. :) [2008-11-18 23:16:44] alec at smecher dot bc dot ca Thanks, lsmith and RhodiumToad. FYI, this bug also exists in PDO (I can post reproduce code if it's helpful). [2008-11-18 22:59:44] lsm...@php.net RhodiumToad lsmith: in a parameterized query it's always wrong to use locale-specific delimiters RhodiumToad is also known as Andrew Gierth and is a highly respected expert on #postgresql on freenode. As such I will reopen the bug .. [2008-10-31 18:28:57] alec at smecher dot bc dot ca FYI, there's a discussion of the same bug, which also appeared (in a separate implementation) in the implementation of the Pear::DB package: http://pear.php.net/bugs/bug.php?id=3021 The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/bug.php?id=46408 -- Edit this bug report at http://bugs.php.net/bug.php?id=46408edit=1
