Everything

php

[RFC] PDO Float Type

php.internals

Adam Baratz

9 years ago

Hi,

The PDO extension does not have a type to represent floating point values.
The current recommended practice is to use PDO::PARAM_STR.

I had poked at this topic in an earlier thread:
https://externals.io/thread/551

There was some hesitation about how complicated this would be to implement.
After looking through each of the supported drivers, it seems like it would
actually be a fairly light lift. In some cases, switching PDO::PARAM_STR
for a new float type constant will save a type cast and have the same
results.

I wrote up a proposal here:
https://wiki.php.net/rfc/pdo_float_type

I tried to be as thorough as possible in understanding the impact on each
supported driver. I'd appreciate any feedback on this concept as well as
its impact on drivers.

Thanks,
Adam

Matteo Beccati

9 years ago

Hi Adam,

On 05/04/2017 17:30, Adam Baratz wrote:

> Hi,
>
> The PDO extension does not have a type to represent floating point values.
> The current recommended practice is to use PDO::PARAM_STR.
>
> I had poked at this topic in an earlier thread:
> https://externals.io/thread/551
>
> There was some hesitation about how complicated this would be to implement.
> After looking through each of the supported drivers, it seems like it would
> actually be a fairly light lift. In some cases, switching PDO::PARAM_STR
> for a new float type constant will save a type cast and have the same
> results.
>
> I wrote up a proposal here:
> https://wiki.php.net/rfc/pdo_float_type
>
> I tried to be as thorough as possible in understanding the impact on each
> supported driver. I'd appreciate any feedback on this concept as well as
> its impact on drivers.

Thanks for that. I generally have very little use for float types on a
database, but I guess their support should have been included from day 1
in PDO.

That said, I think the proposed type is likely to be misused for
NUMERIC/DECIMAL fields, which would be pretty bad. Maybe we should also
add PDO::PARAM_NUMERIC in order to avoid mistakes?


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Adam Baratz

9 years ago

>
> That said, I think the proposed type is likely to be misused for
> NUMERIC/DECIMAL fields, which would be pretty bad. Maybe we should also
> add PDO::PARAM_NUMERIC in order to avoid mistakes?
>

Just so I understand your concern, it's that fixed-precision types are
meaningfully different and there could be clashes with other types of
floats? I agree with you on that, but I'm not sure what the right solution
is. There isn't a C type for fixed-precision floats, or even a PHP type.
How would the flow of data work so nothing's lost/altered along the way? My
general thought would be that if fixed-precision matters, then you should
be storing values as strings, that there'd be a whole other set of pitfalls
opened up with a PDO::PARAM_NUMERIC type.

I realize this is all downsides, which isn't the most constructive way to
respond. Happy to talk through details if you have specific suggestions for
how this type would work in practice.

Thanks,
Adam

Matteo Beccati

9 years ago

Hi Adam,

On 10/04/2017 17:42, Adam Baratz wrote:

> Just so I understand your concern, it's that fixed-precision types are
> meaningfully different and there could be clashes with other types of
> floats? I agree with you on that, but I'm not sure what the right
> solution is. There isn't a C type for fixed-precision floats, or even a
> PHP type. How would the flow of data work so nothing's lost/altered
> along the way? My general thought would be that if fixed-precision
> matters, then you should be storing values as strings, that there'd be a
> whole other set of pitfalls opened up with a PDO::PARAM_NUMERIC type.
>
> I realize this is all downsides, which isn't the most constructive way
> to respond. Happy to talk through details if you have specific
> suggestions for how this type would work in practice.

My concern is that numeric/decimal types should be treated/sent as
strings (and eventually dealt with using e.g. bcmath) and not converted
to floats, unless one seeks trouble and loves rounding errors, while the
new PDO::PARAM_FLOAT const could instead be seen as a good fit to many
unexperienced (and some experienced) people.

Hence my suggestion of a PDO::PARAM_NUMERIC const that could in fact
just be an alias for PDO::PARAM_STR for most of the drivers.

The most important thing to me is to discourage the misuse of
PDO::PARAM_FLOAT for types that are not actually floating points.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Andrew Faulds

9 years ago

Hi,

Matteo Beccati wrote:

> a PDO::PARAM_NUMERIC const that could in fact
> just be an alias for PDO::PARAM_STR for most of the drivers

This is a technical solution to a documentation problem. I think it runs
the risk of confusing people who do know what they're doing, and assume
NUMERIC is like FLOAT.

I'd suggest adding a warning to the manual instead.

--
Andrea Faulds
https://ajf.me/

Matteo Beccati

9 years ago

Hi Andrea,

On 11/04/2017 19:25, Andrea Faulds wrote:

> Matteo Beccati wrote:
>> a PDO::PARAM_NUMERIC const that could in fact just be an alias for
>> PDO::PARAM_STR for most of the drivers
>
> This is a technical solution to a documentation problem.

Yes, and I'd tend to agree. But technically PDO::PARAM_NUMERIC could
also allow to pass parameters in an appropriate format: I believe it
wouldn't be much of an improvement for pdo_pgsql, but to be certain some
level of investigation would be required.

> I think it runs the risk of confusing people who do know what they're
> doing, and assume NUMERIC is like FLOAT.

Hey, that was precisely my point! ;)

I.e. to assume that numeric and float are similar enough that
PDO::PARAM_FLOAT would be a good fit for a numeric field.

I know you meant it the other way around, but to me it is more confusing
to use PARAM_STR for numerics when only PARAM_FLOAT is available as
alternative vs using PARAM_FLOAT on a numeric when both are available.

> I'd suggest adding a warning to the manual instead.

I think that's a requirement in any case.

Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Adam Baratz

9 years ago

>
> > I'd suggest adding a warning to the manual instead.
>
> I think that's a requirement in any case.

I'd be most comfortable with this approach. I'd worry about adding a
PDO::PARAM_NUMERIC type without investigating how it needs to function for
each DB, which feels like scope creep. If it starts off as an alias for
PDO::PARAM_STR, there could be issues updating it to work correctly,
especially if the right design involves modeling the precision somewhere. I
added a "Future Scope" section covering this.

Let me know if there are major problems with this or other points to cover.
Otherwise, I'll aim to open voting on Monday.

Thanks,
Adam

Matteo Beccati

9 years ago

On 12/04/2017 17:21, Adam Baratz wrote:

>>
>>> I'd suggest adding a warning to the manual instead.
>>
>> I think that's a requirement in any case.
>
>
> I'd be most comfortable with this approach. I'd worry about adding a
> PDO::PARAM_NUMERIC type without investigating how it needs to function for
> each DB, which feels like scope creep. If it starts off as an alias for
> PDO::PARAM_STR, there could be issues updating it to work correctly,
> especially if the right design involves modeling the precision somewhere. I
> added a "Future Scope" section covering this.
>
> Let me know if there are major problems with this or other points to cover.
> Otherwise, I'll aim to open voting on Monday.

Let's just agree to disagree. I believe they should be investigated and
proposed in a single RFC. Having just one and relying on an obscure
documentation warning is not enough IMHO.

Even your RFC claims that it should be used for numeric types :(

"This test was repeated using the numeric type for the number column."


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Adam Baratz

9 years ago

>
> > I'd be most comfortable with this approach. I'd worry about adding a
> > PDO::PARAM_NUMERIC type without investigating how it needs to function
> for
> > each DB, which feels like scope creep. If it starts off as an alias for
> > PDO::PARAM_STR, there could be issues updating it to work correctly,
> > especially if the right design involves modeling the precision
> somewhere. I
> > added a "Future Scope" section covering this.
> >
> > Let me know if there are major problems with this or other points to
> cover.
> > Otherwise, I'll aim to open voting on Monday.
>
> Let's just agree to disagree. I believe they should be investigated and
> proposed in a single RFC. Having just one and relying on an obscure
> documentation warning is not enough IMHO.

I looked more closely at each of the APIs. My conclusion was that a single
type will be appropriate for floats, doubles, and fixed-precision. I
updated the RFC with details. If it's accepted, it could be worth including
some of this content in the documentation so people better understand the
impact of each PDO param type.

I'll create a separate thread to announce the vote.

Thanks,
Adam

Matteo Beccati

9 years ago

Hi Adam,

On 18/04/2017 18:58, Adam Baratz wrote:

> I looked more closely at each of the APIs. My conclusion was that a single
> type will be appropriate for floats, doubles, and fixed-precision. I
> updated the RFC with details. If it's accepted, it could be worth including
> some of this content in the documentation so people better understand the
> impact of each PDO param type.
>
> I'll create a separate thread to announce the vote.

Thanks for allowing everyone the time to look at the RFC changes before
opening the vote ;)

Anyway, your suggestion that a single floating point type is appropriate
for both fixed-precision and floating points seems ill-advised, as is
probably the API you are basing your decisions off of.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/