Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121576
MIME-Version: 1.0
References: <734FB3C6-EBE6-4F35-8738-436AF7D8B161@sakiot.com>
In-Reply-To: <734FB3C6-EBE6-4F35-8738-436AF7D8B161@sakiot.com>
Date: Sat, 4 Nov 2023 15:32:25 +0700
Message-ID: <CANLbj-r3A37mRgp4JGPM0ZTuXg1Tdp660c3LBA-QubHPFjq7bg@mail.gmail.com>
To: Saki Takamachi <saki@sakiot.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Fix the inconsistent behavior of PDO::PARAM_XXX
From: ayesh@php.watch (Ayesh Karunaratne)

>
> Hi internals,
>
> As shown in the following issue, the behavior of `PDO::PARAM_XXXX` is inc=
onsistent 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 imple=
mentation 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 ou=
t 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 exi=
sting behavior, I change the behavior of other types. Because LOB have comp=
lex 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 woul=
d also require changing the behavior of the emulator, and I would not be ab=
le to issue a PR for each driver, making the scope too large. For this as w=
ell, I will just align it to `int(1)` or `string(1) 't'`, etc., and once th=
ese 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.