Closed SakiTakamachi closed 1 month ago
SakiTakamachi
commented
10 months ago I forgot to write the important test code.
<?php
$db = new PDO(/* ... */);
$stmt = $db->prepare('SELECT :val as val');
$stmt->bindValue(':val', $val, {PDO::PARAM_XXX});
$stmt->execute();
$row = $stmt->fetch(PDO::FETCH_ASSOC);
var_dump($row['val']);
KentarouTakeda
commented
10 months ago (Highest priority) If the parameter or type specification is NULL, pass NULL
In my opinion, we should pass null.
Certainly, considering the behavior of the application, if a type is specified, it may be appropriate to convert null to some value (empty string or 0) that corresponds to the specified type.
But this feature interacts with the database. Many of the places we interact with use a Three-Valued Logic system, so it is necessary to clearly distinguish between null and other values.
mvorisek
commented
10 months ago I would expect a warning when non-numeric string is passed using PARAM_INT/PARAM_BOOL, and the same if not-null is passed using PARAM_NULL.
SakiTakamachi
commented
10 months ago I think we'll probably need the RFC for warnings because it will significantly change the user experience.
SakiTakamachi
commented
10 months ago I posted about this issue on the mailing list.
Ayesh
commented
10 months ago @SakiTakamachi do you think PHP's type coercing had anything to do with the results?
Looking at the PDOStatement::bindValue signature, it says it only accepts string|int values. So with strict_types=1, passing boolean/null/object values should cause a \TypeError.
I also think the PDO::PARAM_XXX constants were added before we had scalar types. Back then, it might have made sense to introduce these constants, but for new code, I think we are probably better off introducing new methods, such as PDOStatement::bindStringValue, bindBoolValue, bindIntValue, etc. This way, we don't introduce any backward compatibility issues (they are new methods), and we can rely on the existing type system to communicate and enforce types.
SakiTakamachi
commented
10 months ago @Ayesh
Is that the first argument you're talking about? If so, it's an argument (such as :val) that indicates the name or position of the parameter. The value is the second argument and the signature is mixed.
Although it seems reasonable to add new methods, it is note that we will be adding at least 10 new methods in total if do.
Ayesh
commented
10 months ago @SakiTakamachi I'm sorry I was indeed looking at the wrong parameter. The parameter value is indeed mixed. Please completely disregard my point about the inconsistency caused by PHP's type coercing.
Although it seems reasonable to add new methods, it is note that we will be adding at least 10 new methods in total if do.
Good point, I don't think it makes sense to overwhelm the class with several new methods either, not to mention the complexity implementing them in all PDO drivers.
devnexen
commented
10 months ago Let's discuss all of it in the ML, cheers.
SakiTakamachi
commented
10 months ago (It's too big to write on the mailing list, so I'll post a comment for reference.) I tried various cases with the DB that I currently have available.
CREATE TABLE test2 (val varchar(10));
CREATE TABLE test2 (val int);
CREATE TABLE test2 (val BOOLEAN); or CREATE TABLE test2 (val BIT) // for SqlServerINSERT INTO test2 VALUES(null);
INSERT INTO test2 VALUES(0);
INSERT INTO test2 VALUES(8);
INSERT INTO test2 VALUES('');
INSERT INTO test2 VALUES('0');
INSERT INTO test2 VALUES('10');
INSERT INTO test2 VALUES('7str');
INSERT INTO test2 VALUES('str');
INSERT INTO test2 VALUES(true);
INSERT INTO test2 VALUES(false);| DB | null | 0 | 8 | '' | '0' | '10' | '7str' | 'str' | true | false |
|---|---|---|---|---|---|---|---|---|---|---|
| MySQL | NULL | "0" | "8" | "" | "0" | "10" | "7str" | "str" | "1" | "0" |
| PostgreSQL | NULL | "0" | "8" | "" | "0" | "10" | "7str" | "str" | "true" | "false" |
| SQLite3 | NULL | "0" | "8" | "" | "0" | "10" | "7str" | "str" | "1" | "0" |
| SqlServer | NULL | "0" | "8" | "" | "0" | "10" | "7str" | "str" | error [*1] | error [*1] |
| Firebird | NULL | "0" | "8" | "" | "0" | "10" | "7str" | "str" | "TRUE" | "FALSE" |
[*1] Invalid column name {value}.
| DB | null | 0 | 8 | '' | '0' | '10' | '7str' | 'str' | true | false |
|---|---|---|---|---|---|---|---|---|---|---|
| MySQL | NULL | 0 | 8 | error [*1] | 0 | 10 | error [*2] | error [*1] | 1 | 0 |
| PostgreSQL | NULL | 0 | 8 | error [*3] | 0 | 10 | error [*3] | error [*3] | error [*4] | error [*4] |
| SQLite3 | NULL | 0 | 8 | "" | 0 | 10 | "7str" | "str" | 1 | 0 |
| SqlServer | NULL | 0 | 8 | 0 | 0 | 10 | error [*5] | error [*5] | error [*6] | error [*6] |
| Firebird | NULL | 0 | 8 | error [*7] | 0 | 10 | error [*7] | error [*7] | error [*8] | error [*8] |
[1] ERROR 1366 (HY000): Incorrect integer value: {value} for column 'val' at row 1
[2] ERROR 1265 (01000): Data truncated for column 'val' at row 1
[3] ERROR: invalid input syntax for type integer: {value}
[4] ERROR: column "val" is of type integer but expression is of type boolean
[5] Conversion failed when converting the varchar value {value} to data type bit.
[6] Invalid column name {value}.
[7] conversion error from string {value}
[8] conversion error from string "BOOLEAN"
| DB | null | 0 | 8 | '' | '0' | '10' | '7str' | 'str' | true | false |
|---|---|---|---|---|---|---|---|---|---|---|
| MySQL | NULL | 0 | 8 | error [*1] | 0 | 10 | error [*2] | error [*1] | 1 | 0 |
| PostgreSQL | NULL | error[*3] | error[*3] | error [*4] | false | error [*4] | error [*4] | error [*1] | true | false |
| SQLite3 | NULL | 0 | 8 | error [*1] | 0 | 10 | error [*2] | error [*1] | 1 | 0 |
| SqlServer | NULL | 0 | 1 | 0 | 0 | 1 | error [*5] | error [*5] | error [*6] | error [*6] |
| Firebird | NULL | error [*7] | error [*7] | error [*7] | error [*7] | error [*7] | error [*7] | error [*7] | true | false |
[1] ERROR 1366 (HY000): Incorrect integer value: {value} for column 'val' at row 1
[2] ERROR 1265 (01000): Data truncated for column 'val' at row 1
[3] ERROR: column "val" is of type boolean but expression is of type integer
[4] ERROR: invalid input syntax for type boolean: {value}
[5] Conversion failed when converting the varchar value {value} to data type bit.
[6] Invalid column name {value}.
[*7] conversion error from string {value}
SakiTakamachi
commented
10 months ago @kamil-tekiela Was there something bad about it...?
kamil-tekiela
commented
10 months ago These are all different databases. You can't expect them all to work the same.
To be honest, I lost track of what you are trying to fix. We are not going to change the behavior of database servers. PDO drivers are free to implement binding however is most sensible for the given RDBMS. There should be no comparison of binding between different PDO drivers. If there's an issue with any given driver, focus on it. Don't change the whole way PDO works.
On Sun, Nov 5, 2023, 15:46 Saki Takamachi @.***> wrote:
@kamil-tekiela https://github.com/kamil-tekiela Was there something bad about it...?
— Reply to this email directly, view it on GitHub https://github.com/php/php-src/issues/12603#issuecomment-1793757686, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSHGGDKISS7UNOFRLHQMZTYC6RFVAVCNFSM6AAAAAA64HCWWKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJTG42TONRYGY . You are receiving this because you were mentioned.Message ID: @.***>
SakiTakamachi
commented
10 months ago @kamil-tekiela
These are all different databases. You can't expect them all to work the same.
Don't change the whole way PDO works.
That's not my intention. I'm trying to fix something that is clearly not working as a spec. In order to consider the behavior when casting types on the php side, I needed to understand how types are handled on the DB side.
For example, pdo_pgsql currently completely ignores PARAM_INT, but the above verification shows that if you simply implement it to support PARAM_INT, an error will occur if the actual column is boolean when specifying PARAM_INT.
The above verification is not a comparison between DBs, but rather to understand the characteristics of each DB.
cmb69
commented
1 month ago For example, pdo_pgsql currently completely ignores
PARAM_INT, but the above verification shows that if you simply implement it to supportPARAM_INT, an error will occur if the actual column is boolean when specifyingPARAM_INT.
SakiTakamachi
commented
1 month ago Yes, I understand now that there are indeed various issues with this. I will close this issue for now.
Description
This is a problem I found while investigating #12581
The behavior when using
PDO::PARAM_XXXdiffers depending on the driver, and also depends on the combination of parameter type and specified type. The behavior also differs depending on whether you are in emulation mode or not. The survey results are described below. I've decided that it's a bug, so I'm posting it as an issue, but if the mailing list is more appropriate, please let me know.notes: It seems that pdo_odbc does not support emulation mode.
unspecified or PARAM_STR
PARAM_INT
[*1]
SQLSTATE[HY090]: Invalid string or buffer length: [FreeTDS][SQL Server]Invalid string or buffer length (SQLExecute[0] at /php-src/ext/pdo_odbc/odbc_stmt.c:263) // my envPARAM_BOOL
PARAM_NULL
I would like to make the behavior consistent, but what do you think of the following rules?
....What is this argument for in the first place?
Regards.
PHP Version
any
Operating System
any