Dorin Marcoci Added Firebrid native type mapping for integers (smallint, integer, bigint) in PDO_Firebird driver

8 years ago by marius adrian popa — view source

unread

Dorin Marcoci Added
https://github.com/madorin/php-src/tree/fb_nativeFirebrid
native type mapping for integers (smallint, integer, bigint) in
PDO_Firebird driver

Testing is welcome! You need to compile his own branch

Related PDO_Firebird feature request https://bugs.php.net/bug.php?id=72583

8 years ago by Adam Baratz — view source

unread

https://github.com/madorin/php-src/tree/fb_nativeFirebrid
native type mapping for integers (smallint, integer, bigint) in
PDO_Firebird driver

It looks like this is an "always on" feature. There's a PDO attribute,
PDO::ATTR_STRINGIFY_FETCHES, that's intended to allow toggling. Since this
is a functionality change, I'd suggest supporting it. It'll make it easier
for users to migrate if they have any baked-in assumptions around getting
strings back.

Thanks,
Adam

8 years ago by Anatol Belski — view source

unread

Hi,

-----Original Message-----
From: Adam Baratz [mailto:adambaratz@php.net]
Sent: Friday, January 6, 2017 7:59 PM
To: marius adrian popa mapopa@gmail.com
Cc: PHP Developers Mailing List internals@lists.php.net
Subject: Re: [PHP-DEV] Dorin Marcoci Added Firebrid native type mapping for
integers (smallint, integer, bigint) in PDO_Firebird driver

https://github.com/madorin/php-src/tree/fb_nativeFirebrid
native type mapping for integers (smallint, integer, bigint) in
PDO_Firebird driver

It looks like this is an "always on" feature. There's a PDO attribute,
PDO::ATTR_STRINGIFY_FETCHES, that's intended to allow toggling. Since this is
a functionality change, I'd suggest supporting it. It'll make it easier for users to
migrate if they have any baked-in assumptions around getting strings back.

Good catch. Dorin, would you be up to implement this please?

Thanks

Anatol

8 years ago by Dorin Marcoci — view source

unread

Anatol,

As I saw in source code, this flag is handled by PDO base class (and I think it should be in base instead of stingifying it in all driver implementations).
Not sure if this place do the magic, but I'll check and come back: https://github.com/php/php-src/blob/master/ext/pdo/pdo_stmt.c#L656

Dorin Marcoci

-----Original Message-----
From: Anatol Belski [mailto:anatol.php@belski.net]
Sent: Friday, January 6, 2017 9:13 PM
To: 'Adam Baratz' adambaratz@php.net; 'marius adrian popa' mapopa@gmail.com
Cc: 'PHP Developers Mailing List' internals@lists.php.net; 'Dorin Marcoci' dorin.marcoci@marcodor.com
Subject: RE: [PHP-DEV] Dorin Marcoci Added Firebrid native type mapping for integers (smallint, integer, bigint) in PDO_Firebird driver

Hi,

-----Original Message-----
From: Adam Baratz [mailto:adambaratz@php.net]
Sent: Friday, January 6, 2017 7:59 PM
To: marius adrian popa mapopa@gmail.com
Cc: PHP Developers Mailing List internals@lists.php.net
Subject: Re: [PHP-DEV] Dorin Marcoci Added Firebrid native type
mapping for integers (smallint, integer, bigint) in PDO_Firebird
driver

https://github.com/madorin/php-src/tree/fb_nativeFirebrid
native type mapping for integers (smallint, integer, bigint) in
PDO_Firebird driver

It looks like this is an "always on" feature. There's a PDO attribute,
PDO::ATTR_STRINGIFY_FETCHES, that's intended to allow toggling. Since
this is a functionality change, I'd suggest supporting it. It'll make
it easier for users to migrate if they have any baked-in assumptions around getting strings back.

Good catch. Dorin, would you be up to implement this please?

Thanks

Anatol

8 years ago by Dorin Marcoci — view source

unread

Yep, setting this flag, integers became strings, so all OK, we have a toggle.
A nice weekend to all! :)

Hi,

-----Original Message-----
From: Adam Baratz [mailto:adambaratz@php.net]
Sent: Friday, January 6, 2017 7:59 PM
To: marius adrian popa mapopa@gmail.com
Cc: PHP Developers Mailing List internals@lists.php.net
Subject: Re: [PHP-DEV] Dorin Marcoci Added Firebrid native type
mapping for integers (smallint, integer, bigint) in PDO_Firebird
driver

https://github.com/madorin/php-src/tree/fb_nativeFirebrid
native type mapping for integers (smallint, integer, bigint) in
PDO_Firebird driver

It looks like this is an "always on" feature. There's a PDO attribute,
PDO::ATTR_STRINGIFY_FETCHES, that's intended to allow toggling. Since
this is a functionality change, I'd suggest supporting it. It'll make
it easier for users to migrate if they have any baked-in assumptions around getting strings back.

Good catch. Dorin, would you be up to implement this please?

Thanks

Anatol

8 years ago by marius adrian popa — view source

unread

Could you add also type mapping for float , double ...

On Fri, Jan 6, 2017 at 9:23 PM, Dorin Marcoci dorin.marcoci@marcodor.com
wrote:

Yep, setting this flag, integers became strings, so all OK, we have a
toggle.
A nice weekend to all! :)

-----Original Message-----
From: Anatol Belski [mailto:anatol.php@belski.net]
Sent: Friday, January 6, 2017 9:13 PM
To: 'Adam Baratz' adambaratz@php.net; 'marius adrian popa' <
mapopa@gmail.com>
Cc: 'PHP Developers Mailing List' internals@lists.php.net; 'Dorin
Marcoci' dorin.marcoci@marcodor.com
Subject: RE: [PHP-DEV] Dorin Marcoci Added Firebrid native type mapping
for integers (smallint, integer, bigint) in PDO_Firebird driver

Hi,

-----Original Message-----
From: Adam Baratz [mailto:adambaratz@php.net]
Sent: Friday, January 6, 2017 7:59 PM
To: marius adrian popa mapopa@gmail.com
Cc: PHP Developers Mailing List internals@lists.php.net
Subject: Re: [PHP-DEV] Dorin Marcoci Added Firebrid native type
mapping for integers (smallint, integer, bigint) in PDO_Firebird
driver

https://github.com/madorin/php-src/tree/fb_nativeFirebrid
native type mapping for integers (smallint, integer, bigint) in
PDO_Firebird driver

It looks like this is an "always on" feature. There's a PDO attribute,
PDO::ATTR_STRINGIFY_FETCHES, that's intended to allow toggling. Since
this is a functionality change, I'd suggest supporting it. It'll make
it easier for users to migrate if they have any baked-in assumptions
around getting strings back.

Good catch. Dorin, would you be up to implement this please?

Thanks

Anatol

8 years ago by Dorin Marcoci — view source

unread

Hello Marius,

Unfortunately with current PDO state this is not possible to be done in a nice way.
Now PDO have defined just these field types: PDO::PARAM_BOOL, PDO::PARAM_INT, PDO::PARAM_STR, PDO::PARAM_LOB, PDO::PARAM_NULL.
Last one is “wrong” and should be removed. NULL is a state, not a type or a value. See #73852 bug.
So PDO should be extended with at least PDO::PARAM_FLOAT to add floats/double mapping. It needs an RFC to be extended.
In plans is to add yet for booleans, Firebird3 bool data type.

Dorin Marcoci

From: marius adrian popa [mailto:mapopa@gmail.com]
Sent: Tuesday, January 10, 2017 3:33 PM
To: Dorin Marcoci dorin.marcoci@marcodor.com
Cc: Anatol Belski anatol.php@belski.net; Adam Baratz adambaratz@php.net; PHP Developers Mailing List internals@lists.php.net
Subject: Re: [PHP-DEV] Dorin Marcoci Added Firebrid native type mapping for integers (smallint, integer, bigint) in PDO_Firebird driver

Could you add also type mapping for float , double ...

Yep, setting this flag, integers became strings, so all OK, we have a toggle.
A nice weekend to all! :)

-----Original Message-----
From: Anatol Belski [mailto:mailto:anatol.php@belski.net]
Sent: Friday, January 6, 2017 9:13 PM
To: 'Adam Baratz' mailto:adambaratz@php.net; 'marius adrian popa' mailto:mapopa@gmail.com
Cc: 'PHP Developers Mailing List' mailto:internals@lists.php.net; 'Dorin Marcoci' mailto:dorin.marcoci@marcodor.com
Subject: RE: [PHP-DEV] Dorin Marcoci Added Firebrid native type mapping for integers (smallint, integer, bigint) in PDO_Firebird driver

Hi,

-----Original Message-----
From: Adam Baratz [mailto:mailto:adambaratz@php.net]
Sent: Friday, January 6, 2017 7:59 PM
To: marius adrian popa mailto:mapopa@gmail.com
Cc: PHP Developers Mailing List mailto:internals@lists.php.net
Subject: Re: [PHP-DEV] Dorin Marcoci Added Firebrid native type
mapping for integers (smallint, integer, bigint) in PDO_Firebird
driver

https://github.com/madorin/php-src/tree/fb_nativeFirebrid
native type mapping for integers (smallint, integer, bigint) in
PDO_Firebird driver

It looks like this is an "always on" feature. There's a PDO attribute,
PDO::ATTR_STRINGIFY_FETCHES, that's intended to allow toggling. Since
this is a functionality change, I'd suggest supporting it. It'll make
it easier for users to migrate if they have any baked-in assumptions around getting strings back.

Good catch. Dorin, would you be up to implement this please?

Thanks

Anatol

8 years ago by Adam Baratz — view source

unread

Unfortunately with current PDO state this is not possible to be done in a
nice way.
Now PDO have defined just these field types: PDO::PARAM_BOOL,
PDO::PARAM_INT, PDO::PARAM_STR, PDO::PARAM_LOB, PDO::PARAM_NULL.
Last one is “wrong” and should be removed. NULL is a state, not a type or
a value. See #73852 bug.
So PDO should be extended with at least PDO::PARAM_FLOAT to add
floats/double mapping. It needs an RFC to be extended.
In plans is to add yet for booleans, Firebird3 bool data type.

Column data can be treated differently from parameter types. I went through
this exercise with pdo_dblib:
https://github.com/php/php-src/blob/master/ext/pdo_dblib/dblib_stmt.c#L254

It's a little clunky to deal with the pointers correctly, but if Firebird
reports a type, you might as well honor it.

I didn't realize until you posted this change that PDO core would cast to
ints in some cases. It made me wonder if that code should deal with other
types or whether all type handling should be pushed into the drivers.
Definitely a separate topic, though...

Regarding additional parameter types, it's on my todo to write up an RFC to
cover some ambiguous cases:

Binary data (see https://bugs.php.net/bug.php?id=47982)
Floating point numbers (see https://www.mail-archive.
com/internals@lists.php.net/msg89591.html
https://www.mail-archive.com/internals@lists.php.net/msg89591.html)
Unicode (touched on with
internals@lists.php.net/msg88713.html" rel="nofollow" target="_blank">https://www.mail-archive.com/internals@lists.php.net/msg88713.html)

My perspective is mainly as someone who works with pdo_dblib. Emulated
prepares make a greater degree of control more desirable in some cases. I
need to think through how such types would impact other drivers, and I've
unfortunately been short on time for the exercise.

Adam

8 years ago by Dorin Marcoci — view source

unread

Hello Adam,

That's why I wrote " is not possible to be done it in a nice way", where "nice" is the key :)
Yes, currently it can be done by using ZVALs, but I find it over-layered & over-structured than simply pass a zend_long for example in case of ints.
As I understand ZVAL purpose in PHP is a bit different. Later in fetch_value ZVAL is copied into another, even not used further. A lot of unused operations.

About floating point numbers (msg89591.html), yes, great move. I'm with you and vote "yes".
Maybe just name it PDO::PARAM_FLOAT instead of DOUBLE to be more in sync with PHP native types, as "double" describe more a size than a type.

About unicode (msg88713.html) I think it’s a wrong way. It should be just strings/(var)chars as in PHP/DBMSes.
We should instead improve/standardize getColumnMeta where we can return charset/collate, for other types numeric precision etc...

Thank You,

Dorin Marcoci