Honor PDO::PARAM_STR_CHAR in real prepared statements - #1686
Open
Ethan Setnik (esetnik) wants to merge 1 commit into
Open
Ethan Setnik (esetnik) wants to merge 1 commit into
Ethan Setnik (esetnik) wants to merge 1 commit into
Conversation
PDO_SQLSRV declared every UTF-8 string parameter of a real prepared statement as nvarchar and ignored PDO::PARAM_STR_CHAR and PDO::ATTR_DEFAULT_STR_PARAM, which emulated prepared statements and PDO::quote() already honor. Comparing an nvarchar parameter with a varchar column makes SQL Server convert the column (CONVERT_IMPLICIT), which prevents index seeks on it and hides its statistics. Declare such a parameter as varchar when it is marked PDO::PARAM_STR_CHAR, or when PDO::ATTR_DEFAULT_STR_PARAM is PDO::PARAM_STR_CHAR and the binding does not override it with PDO::PARAM_STR_NATL. The value is still sent as UTF-16 and the ODBC driver converts it to the server code page, as in the emulated case. Applications that set neither keep the nvarchar binding. Addresses microsoft#1587. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Author
|
@microsoft-github-policy-service agree company="Prodigy EMS" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #1587. Related: #1540.
Problem
In real prepared statements, PDO_SQLSRV declares every UTF-8 string parameter as
nvarcharand deliberately ignoresPDO::PARAM_STR_CHARandPDO::ATTR_DEFAULT_STR_PARAM(pdo_stmt.cpplogs "set but is ignored"), although emulated prepared statements andPDO::quote()honor both. Comparing annvarcharparameter with avarcharcolumn makes SQL Server convert the column (CONVERT_IMPLICIT). Under a SQL collation such as the defaultSQL_Latin1_General_CP1_CI_AS, that rules out a seek on an index keyed on the column, and under any collation it degrades the optimizer's cardinality estimates for the predicate.Change
For an input
PDO::PARAM_STRstring parameter whose effective encoding is UTF-8, declare it asSQL_VARCHARwhen:PDO::PARAM_STR_CHAR, orPDO::ATTR_DEFAULT_STR_PARAMisPDO::PARAM_STR_CHARand the binding does not carryPDO::PARAM_STR_NATL.Only the declared SQL type changes. The value is still converted to UTF-16 and sent as
SQL_C_WCHAR, and the ODBC driver converts it to the server code page, which is the same semantics as the emulated path. Size derivation is unchanged (varchar(4000), orvarchar(max)above 8000 bytes of UTF-16), so long values are not truncated (covered by the new test).Unchanged: the default (neither option set),
SQLSRV_ENCODING_SYSTEMandSQLSRV_ENCODING_BINARYparameters, output and input/output parameters, LOBs and streams, emulated prepares, Always Encrypted (the core still replaces the type with the encrypted column's metadata), and the SQLSRV extension (no shared code changed).Why the standard PDO flags
They need no new API, and they reach
PDOStatement::bindValue(), which has no$driver_optionsparameter. Laravel's query builder and Doctrine DBAL's PDO driver bind every value withbindValue(), so abindParam()driver option (as discussed in #1540) cannot reach them, whilePDO::ATTR_DEFAULT_STR_PARAMcan be set from their connection configuration. Doctrine DBAL also already hasParameterType::ASCII, which its PDO driver currently maps to plainPDO::PARAM_STR; with this change it could map it toPDO::PARAM_STR | PDO::PARAM_STR_CHARand getvarcharbinding. This complements per-parameter driver options rather than competing with them.Behavior change
Code that already sets
PDO::PARAM_STR_CHAR, orPDO::ATTR_DEFAULT_STR_PARAMtoPDO::PARAM_STR_CHAR, now getsvarcharbinding in real prepares too, so characters the server code page cannot represent are replaced, exactly as they already are with emulated prepares.pdo_1018_real_prepare_natl_char.phptlocked in the old behavior; its UTF-8 case 1 now expects the replaced value. With a_UTF8collation (the #1587 scenario) the code page is UTF-8 and the round trip is lossless; see below. If you would prefer this behind an opt-in connection attribute, I am happy to add one.Evidence
Ubuntu 24.04 (amd64), PHP 8.4.26, ODBC Driver 18.7.1.1, unixODBC 2.3.12, SQL Server 2022 16.0.4222.2 (
SQL_Latin1_General_CP1_CI_AS). LookupSELECT id FROM t WHERE code = ?on 200,000 rows with an index oncode varchar(20), actual plans viaSET STATISTICS XML ON:CONVERT_IMPLICITon columndevPARAM_STRdevPARAM_STR | PARAM_STR_CHARPARAM_STRPARAM_STR | PARAM_STR_CHAR_UTF8collation (database created withLatin1_General_100_CI_AS_SC_UTF8,varchar(100)column), inserting CJK text plus an emoji: withPARAM_STR | PARAM_STR_CHARthe parameter is declaredvarcharand the value reads back unchanged.Tests
pdo_1587_real_prepare_param_str_char.phpt: checks the declared type of each parameter throughSQL_VARIANT_PROPERTY(CAST(? AS sql_variant), 'BaseType')forbindValue()andbindParam(), the per-binding flags, bothPDO::ATTR_DEFAULT_STR_PARAMvalues,PARAM_STR_NATLoverriding aPARAM_STR_CHARdefault, explicit system/binary/UTF-8 parameter encodings, a non-string parameter, and a 9,000-byte value sent in full. It fails ondev(everyPARAM_STR_CHARcase reportsnvarchar) and passes with this change.pdo_1018_real_prepare_natl_char.phptas described above.pdo_sqlsrvfunctional suite in the environment above, with this change: 322 passed, 0 failed, 18 skipped (PHP 7-only, secure enclave, Windows-only, Azure Key Vault credentials, the mock TDS server, and one test whose skip check could not connect). Ondev, the same run fails only the two tests above.pdo_connection_resiliency*.phpt,pdo_678_conn_resiliency_pooling.phpt) fail intermittently in this emulated amd64 environment on bothdevand this branch (TCP Provider: Error code 0x20after the test kills its session), so I have treated them as environmental.pdo_stmt.cppwith-Wall -Wextra. I have not built on Windows or macOS; CI should cover those.🤖 Generated with Claude Code