Everything

php

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

php.internals

marius popa

9 years ago

Dorin Marcoci Added
<https://github.com/madorin/php-src/tree/fb_native>Firebrid
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

Adam Baratz

9 years ago

>
> <https://github.com/madorin/php-src/tree/fb_native>Firebrid
> 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

Anatol Belski

9 years ago

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_native>Firebrid
> > 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

Dorin Marcoci

9 years ago

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_native>Firebrid
> > 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

Dorin Marcoci

9 years ago

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_native>Firebrid
> > 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

marius popa

9 years ago

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:

Dorin Marcoci

9 years ago

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 ...

On Fri, Jan 6, 2017 at 9:23 PM, Dorin Marcoci <mailto: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: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_native>Firebrid
> > 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

Adam Baratz

9 years ago

>
> 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
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

Dorin Marcoci

9 years ago

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