# Fix the inconsistent behavior of PDO::PARAM_XXX

- Thread: https://externals.io/message/121575
- Original archive: https://news-web.php.net/php.internals/121575
- Started by Saki Takamachi on 2023-11-04T06:59:24+00:00
- Messages: 18
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #121575 — Saki Takamachi — 2023-11-04T06:59:24+00:00

URL: https://externals.io/message/121575#121575

Hi internals,

As shown in the following issue, the behavior of `PDO::PARAM_XXXX` is inconsistent and I would like to fix this.
https://github.com/php/php-src/issues/12603

First, I tried fixed pdo_pgsql.
https://github.com/php/php-src/pull/12605

Eventually I plan to make similar changes to all bundled pdos.

What do you think about these? If there is a reason for the current implementation that should not be changed, it would be very helpful if you could tell me why.

Also, if an RFC is required, it would be helpful if you could point it out as well. Personally, I don't think an RFC is necessary since this is some kind of bug fix. However, I believe the target should be the master branch as it changes the user experience somewhat.

[*] I'm not thinking about LOB here, but once I leave them with their existing behavior, I change the behavior of other types. Because LOB have complex behavior, the scope becomes too large. After making this change, I will test again and post another issue if necessary.

[*] I would like the type of PARAM_BOOL to be exactly bool, but this would also require changing the behavior of the emulator, and I would not be able to issue a PR for each driver, making the scope too large. For this as well, I will just align it to `int(1)` or `string(1) 't'`, etc., and once these changes are completed, I will verify it and post an issue.

[*] odbc etc. are not compatible with emulation in the first place. There are no plans to change this in this context.

Regards.

Saki

---

## #121576 — Ayesh Karunaratne — 2023-11-04T08:32:25+00:00

URL: https://externals.io/message/121575#121576 · In reply to: #121575

>
> Hi internals,
>
> As shown in the following issue, the behavior of `PDO::PARAM_XXXX` is inconsistent and I would like to fix this.
> https://github.com/php/php-src/issues/12603
>
> First, I tried fixed pdo_pgsql.
> https://github.com/php/php-src/pull/12605
>
> Eventually I plan to make similar changes to all bundled pdos.
>
> What do you think about these? If there is a reason for the current implementation that should not be changed, it would be very helpful if you could tell me why.
>
> Also, if an RFC is required, it would be helpful if you could point it out as well. Personally, I don't think an RFC is necessary since this is some kind of bug fix. However, I believe the target should be the master branch as it changes the user experience somewhat.
>
> [*] I'm not thinking about LOB here, but once I leave them with their existing behavior, I change the behavior of other types. Because LOB have complex behavior, the scope becomes too large. After making this change, I will test again and post another issue if necessary.
>
> [*] I would like the type of PARAM_BOOL to be exactly bool, but this would also require changing the behavior of the emulator, and I would not be able to issue a PR for each driver, making the scope too large. For this as well, I will just align it to `int(1)` or `string(1) 't'`, etc., and once these changes are completed, I will verify it and post an issue.
>
> [*] odbc etc. are not compatible with emulation in the first place. There are no plans to change this in this context.
>
> Regards.
>
> Saki

I also commented on the PR, but in the interest of keeping the
conversation in the mailing list, I'll summarize my thoughts here as
well.

I don't think we should change the existing behavior. It may be an
xkcd/1172 case, but these kinds of cases are very hard to spot from
static analyzers, and are often only surfaced in production only if
someone spent enough time to dig deep. It's not helping when certain
database softwares try to be forgiving and sloppy with types (looking
at you, MySQL).

In the PR, I proposed to add new methods (`bindStringValue`,
`bingBoolValue`, etc) as a non-BC approach, that also rely on the
existing type system to communicate the types and enforce them. Saki
said that it might not be possible to introduce 10+ new methods to
`PDOStatement`, to which I also agree. However, maybe we can
conservatively add the basic four types (BOOL/NULL/INT/STR) as a
start.

---

## #121577 — Saki Takamachi — 2023-11-04T08:55:42+00:00

URL: https://externals.io/message/121575#121577 · In reply to: #121576

Hi, Ayesh,

I forgot to tell you one important thing.

Binding of parameters is not actually done in the `bindXXXX`method, but in the `execute()` method. Therefore, when preparing a new method, a slightly larger mechanism is required to determine which method the parameter was set through.

Regards.

Saki

---

## #121578 — Hans Henrik Bergan — 2023-11-04T08:56:14+00:00

URL: https://externals.io/message/121575#121578 · In reply to: #121575

I had no idea PDO's PARAM_INT and PARAM_BOOL was so buggy, good catch!



> Hi internals,
>
> As shown in the following issue, the behavior of `PDO::PARAM_XXXX` is
> inconsistent and I would like to fix this.
> https://github.com/php/php-src/issues/12603
>
> First, I tried fixed pdo_pgsql.
> https://github.com/php/php-src/pull/12605
>
> Eventually I plan to make similar changes to all bundled pdos.
>
> What do you think about these? If there is a reason for the current
> implementation that should not be changed, it would be very helpful if you
> could tell me why.
>
> Also, if an RFC is required, it would be helpful if you could point it out
> as well. Personally, I don't think an RFC is necessary since this is some
> kind of bug fix. However, I believe the target should be the master branch
> as it changes the user experience somewhat.
>
> [*] I'm not thinking about LOB here, but once I leave them with their
> existing behavior, I change the behavior of other types. Because LOB have
> complex behavior, the scope becomes too large. After making this change, I
> will test again and post another issue if necessary.
>
> [*] I would like the type of PARAM_BOOL to be exactly bool, but this would
> also require changing the behavior of the emulator, and I would not be able
> to issue a PR for each driver, making the scope too large. For this as
> well, I will just align it to `int(1)` or `string(1) 't'`, etc., and once
> these changes are completed, I will verify it and post an issue.
>
> [*] odbc etc. are not compatible with emulation in the first place. There
> are no plans to change this in this context.
>
> Regards.
>
> Saki

---

## #121579 — Saki Takamachi — 2023-11-04T08:59:25+00:00

URL: https://externals.io/message/121575#121579 · In reply to: #121578

Hi Hans,

Thank you, I will discuss it and improve it in a way that everyone is satisfied with.

Saki

---

## #121580 — Kentaro Takeda via internals — 2023-11-04T10:46:28+00:00

URL: https://externals.io/message/121575#121580 · In reply to: #121577

Hi, internals.

> As shown in the following issue, the behavior of `PDO::PARAM_XXXX` is
> inconsistent and I would like to fix this.
> https://github.com/php/php-src/issues/12603

I consider the current behavior a bug.
And some of them contain behaviors that are very confusing to users.

> It may be an xkcd/1172 case, but these kinds of cases are very hard
> to spot from static analyzers, and are often only surfaced
> in production only if someone spent enough time to dig deep.

As previously discussed by Ayesh, assessing
the users' impact of a fix for this issue might be a little challenging,

but I think it's important to address it to prevent confusion.

Taking PostgreSQL as an example, let's consider the following PHP code:

```php
$pdo = new PDO('pgsql:');
$pdo->exec('create temp table t(boolean_column bool, text_column text)');

$stmt = $pdo->prepare('insert into t values (:v1, :v2)');
$stmt->bindValue(':v1', false, PDO::PARAM_BOOL);
$stmt->bindValue(':v2', false, PDO::PARAM_BOOL);
$stmt->execute();

$result = $pdo->query('select * from t');
var_export($result->fetchAll(PDO::FETCH_ASSOC));
````

The result should look like this:

````
array (
   0 =>
   array (
     'boolean_column' => false,
     'text_column' => 'f', // HERE!!!
   ),
)
````

Even if we insert `false`, it will change to `'f'` when we select it.

This is fundamentally a user mistake. However, the big problem in this case is
that the value `'f'` after the change is *truthy*. Who could have predicted
that even though you entered `false`, it ended up being `true` ?

I've seen a lot of pointless code to deal with problems
like this where types are not uniformly formatted.

As Saki says, we need to think carefully about whether
we can accurately treat this as `bool`.
However, regarding the above problem:

* Fix *truthy* / *falsy* such as `int(1)` / `int(0)` to a form that
can be correctly determined.
* Make this behavior consistent across as many drivers as possible.

Even this small fix would eliminate much of the confusion that is
currently occurring.
(Of course, it should ultimately be treated as `bool`.)

I look forward to a resolution that will be satisfactory to all.
Thank you for your attention.

2023年11月4日(土) 17:56 Saki Takamachi <saki@sakiot.com>:
>
> Hi, Ayesh,
>
> I forgot to tell you one important thing.
>
> Binding of parameters is not actually done in the `bindXXXX`method, but in the `execute()` method. Therefore, when preparing a new method, a slightly larger mechanism is required to determine which method the parameter was set through.
>
> Regards.
>
> Saki
>
> --
> 
> To unsubscribe, visit: https://www.php.net/unsub.php

---

## #121582 — Saki Takamachi — 2023-11-04T12:43:55+00:00

URL: https://externals.io/message/121575#121582 · In reply to: #121580

Hi Kentaro,

The case you presented certainly confuses us very much. Although it is outside the scope of this discussion, it certainly seems like it would be better to improve it from a safety perspective.

BOOL probably has the most problems; for example, when you pass str, the boolean value will be false only in sqlite. This happens because str is converted to long and then to bool.

At least for this kind of behavior, I don't think it's necessary to maintain compatibility.

Regards.

Saki

---

## #121586 — Saki Takamachi — 2023-11-05T09:10:11+00:00

URL: https://externals.io/message/121575#121586 · In reply to: #121575

Hi,

To think more deeply about this issue, I investigated various cases in major databases. It's too big to write in an email, so I posted it in the comments of the issue.

https://github.com/php/php-src/issues/12603#issuecomment-1793679776

Regards.

Saki

---

## #121587 — Matteo Beccati — 2023-11-05T12:31:53+00:00

URL: https://externals.io/message/121575#121587 · In reply to: #121575

Hi Saki,

Il 04/11/2023 07:59, Saki Takamachi ha scritto:
> Hi internals,
> 
> As shown in the following issue, the behavior of `PDO::PARAM_XXXX` is inconsistent and I would like to fix this.
> https://github.com/php/php-src/issues/12603
> 
> First, I tried fixed pdo_pgsql.
> https://github.com/php/php-src/pull/12605
> 
> Eventually I plan to make similar changes to all bundled pdos.
> 
> What do you think about these? If there is a reason for the current implementation that should not be changed, it would be very helpful if you could tell me why.

I'm sure there are several bugs w/ types in PDO.

Perhaps some can be fixed, but I would be very careful touching that part.

The last time I tried to fix the PDO_PARAM_INT behaviour on pdo_pgsql I 
broke Laravel and surely many other projects.

I'm afraid that most libraries and projects have now worked around some 
of the bugs and trying to fix them is going to cause BC problems, or 
generate a whole new series of bugs and incompatibilities.

See:

https://bugs.php.net/bug.php?id=80892
https://github.com/php/php-src/pull/6801


Cheers
-- 
Matteo Beccati

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

---

## #121588 — Hans Henrik Bergan — 2023-11-05T12:40:23+00:00

URL: https://externals.io/message/121575#121588 · In reply to: #121586

I think it'd be a good idea if they used `FILTER_VALIDATE_BOOLEAN` and
FILTER_VALIDATE_INTEGER type logic, with an error if conversion fails..

I wonder if PDO::PARAM_BOOL_OR_NULL would be worthwhile



> Hi,
>
> To think more deeply about this issue, I investigated various cases in
> major databases. It's too big to write in an email, so I posted it in the
> comments of the issue.
>
> https://github.com/php/php-src/issues/12603#issuecomment-1793679776
>
> Regards.
>
> Saki
> --
> 
> To unsubscribe, visit: https://www.php.net/unsub.php

---

## #121590 — Saki Takamachi — 2023-11-05T13:52:00+00:00

URL: https://externals.io/message/121575#121590 · In reply to: #121587

Hi Matteo,

> I'm sure there are several bugs w/ types in PDO.
> 
> Perhaps some can be fixed, but I would be very careful touching that part.
> 
> The last time I tried to fix the PDO_PARAM_INT behaviour on pdo_pgsql I broke Laravel and surely many other projects.
> 
> I'm afraid that most libraries and projects have now worked around some of the bugs and trying to fix them is going to cause BC problems, or generate a whole new series of bugs and incompatibilities.

I hate to be the person to tell you this, it is simply a problem caused by releasing broken code. 

However admittedly, we may need a some more test patterns to compensate for not breaking functionality.

Regards.

Saki

---

## #121591 — Saki Takamachi — 2023-11-05T15:22:33+00:00

URL: https://externals.io/message/121575#121591 · In reply to: #121587

I should have said that the spec was broken, not the code. By that logic, my PR is probably broken as well. I did additional research on each DB to see if the specs were broken.

Saki

---

## #121592 — Kentaro Takeda via internals — 2023-11-05T15:36:55+00:00

URL: https://externals.io/message/121575#121592 · In reply to: #121587

Hi, Matteo,

> The last time I tried to fix the PDO_PARAM_INT behavior on pdo_pgsql I broke Laravel and surely many other projects.
> https://github.com/php/php-src/pull/6801

Thank you for providing an important example.

Regarding this past issue with PostgreSQL, it can be solved by
treating numbers larger than `int4` as `unknown` (as is the case now)
rather than as `int8` (as in previous attempts).

This approach aligns with PostgreSQL's capability to cast `integer` to
`boolean` but not `smallint` or `bigint`. This consideration is
crucial for ensuring interoperability.

````
postgres=# select 1::integer::boolean ; -- returns `t`
postgres=# select 1::smallint::boolean ; -- `ERROR: cannot cast type
smallint to boolean`
postgres=# select 1::bigint::boolean ; -- `ERROR: cannot cast type
bigint to boolean`
````

It is certainly not realistic to unify behavior across all databases.
Also, as in the example above, there are certainly places where each
database requires individual support.

I think it would be good if a solution could be found that would allow
for more correct behavior without breaking the behavior of existing
applications.

Your insights and contributions are interesting and invaluable. They
are much appreciated.
Best regards

---

## #121593 — Saki Takamachi — 2023-11-05T16:42:55+00:00

URL: https://externals.io/message/121575#121593 · In reply to: #121587

I couldn't express myself well in English, I'm sure my words were bad. sorry.

Saki

---

## #121594 — Kentaro Takeda via internals — 2023-11-05T16:43:20+00:00

URL: https://externals.io/message/121575#121594 · In reply to: #121592

Hi, Matteo

> Regarding this past issue with PostgreSQL, it can be solved by
> treating numbers larger than `int4` as `unknown` (as is the case now)
> rather than as `int8` (as in previous attempts).

I'm sorry, this was my mistake.

````
select 1::boolean ; -- returns `t`
select 1 = true ; -- `ERROR: operator does not exist: integer = boolean`
````

The conditions for casting and comparison were different.
`PDO_PARAM_INT` still needs to be passed as `unknown` to PostgreSQL.

Thank you!

---

## #121598 — Saki Takamachi — 2023-11-06T12:20:07+00:00

URL: https://externals.io/message/121575#121598 · In reply to: #121588

Hi Hans,

> I think it'd be a good idea if they used `FILTER_VALIDATE_BOOLEAN` and FILTER_VALIDATE_INTEGER type logic, with an error if conversion fails..

This can be difficult. Forcing an error is highly likely to destroy the existing user environment.

> I wonder if PDO::PARAM_BOOL_OR_NULL would be worthwhile 

That's what I thought at first, but I think it might be a good idea to leave it as a fallback, especially for NULL. In fact, there have been proposals to deprecate PARAM_NULL in the past, but none have made it to the voting phase. I have not looked into it in detail yet, but I suspect that during the discussion stage, they may have come to the conclusion that it should not be abolished.

Regards.

Saki

---

## #121599 — Saki Takamachi — 2023-11-06T13:57:39+00:00

URL: https://externals.io/message/121575#121599 · In reply to: #121593

Hi,

I think this is probably the same problem that Matteo ran into, but when using emulate mode, pgsql errors out with code like this:

```
<?php
$db = new PDO(
    pgsql:/* dsn */,
    /* username */,
    /* password */,
    [
        PDO::ATTR_EMULATE_PREPARES => true,
    ],
);

$db->exec('CREATE TABLE test2 (val BOOLEAN)');

$stmt = $db->prepare('INSERT INTO test2 VALUES(:val)');
$stmt->bindValue(':val', 1, PDO::PARAM_INT);
$stmt->execute();
```

```
Fatal error: Uncaught PDOException: SQLSTATE[42804]: Datatype mismatch: 7 ERROR:  column "val" is of type boolean but expression is of type integer
```

It seems a little strange that whether an error occurs or not depends on whether you are in emulation mode or not.

Saki

---

## #121600 — Saki Takamachi — 2023-11-06T14:01:28+00:00

URL: https://externals.io/message/121575#121600 · In reply to: #121599

As Ayesh says, I'm starting to feel like I need to provide a completely new feature separately tbh…

The following issues are completely bugs and should at least be fixed.
https://github.com/php/php-src/issues/12581

Saki