Quote Update SET columns as identifiers and bind them under valid placeholder names

[simon-mundy]

Problem

Update renders SET column names as free-form SQL fragments instead of identifiers. That causes two problems:

1. Quoting. On the SQL-92 platform, set(['x-y' => 1]) renders "x""-""y" = …, set(['a b' => 1]) renders "a" "b" = …, and set(['c"d' => 1]) renders "c""'""d" = ….
2. Parameter names. In prepared mode, a scalar SET value is bound under a parameter named after its column. A dotted column such as film.title binds as :film.title. Any other non-word character has the same effect. Those are not valid named placeholders on most drivers.

Insert has the same parameter-name problem in its VALUES list (:film.title, :x-y). It does not have the quoting problem.

Cause

Update::processSet() resolves each SET column through resolveColumnValue(['isIdentifier' => true]). That ends in quoteIdentifierInFragment(), which splits on every non-word character and quotes each piece. Update::processSet() and Insert::processInsert() both use the raw column name as the parameter name for non-PDO drivers.

Fix

• SET target. Update now quotes the SET target as an identifier chain. It splits only on the table-qualifier ., quotes each part with the platform's quoteIdentifier(), and joins the parts with getIdentifierSeparator(). This keeps the platform's own quote characters, its embedded-quote escaping and its quoteIdentifiers switch.
• Parameter names. The new AbstractSql::resolveParameterNames() maps each column to a valid placeholder name:
  • Names made only of [a-zA-Z0-9_] are kept as they are.
  • In any other name, each other character becomes _.
  • A suffix (_1, _2, …) is added when the result would collide with another column's name. Plain names are reserved first, so they never change, whatever the column order.
• PDO. The c_N names are unchanged.

Behaviour change (non-plain names only)

Input	Before	After
Update set(['x-y' => …])	"x""-""y"	"x-y"
Update set(['a b' => …])	"a" "b"	"a b"
Update set(['c"d' => …])	"c""'""d"	"c'd" (the SQL-92 platform's own quoteIdentifier() escaping)
Prepared parameter for film.title / x-y / a b (non-PDO)	:film.title / :x-y / :a b	:film_title / :x_y / :a_b
set(['a.b' => …, 'a_b' => …])	—	:a_b_1 and :a_b

Plain and dotted SET columns render exactly as before ("title", "film"."title"). Insert column quoting is unchanged.

Tests

test/unit/Sql/UpdateColumnNameTest.php and test/unit/Sql/InsertColumnNameTest.php, both #[Group('unit')]. Data providers cover plain, dotted, hyphen, space, quote-character and colliding names, with assertions on:

• the unprepared SQL;
• the prepared SQL together with getNamedArray();
• PDO c_N naming staying the same.

The tests were written first and failed 18 cases before the fix; the plain cases already passed. Changed lines have 100% line coverage. The full unit suite, mago format --check, mago lint and mago analyze are clean.

Follow-ups (not in this PR)

• Insert column list. Insert quotes each column as a single identifier, so film.title renders "film.title", not "film"."title". Standard SQL doesn't allow qualified INSERT columns (MySQL does), so this is left alone here.
• SQL-92 / SQLite escaping. quoteIdentifierTo is ' in AbstractPlatform (SQL-92) and in phpdb-sqlite's AdapterPlatform. An embedded " therefore becomes ', not the standard "". phpdb-pgsql ("") and phpdb-mysql (backtick, doubled) escape correctly.
• quoteIdentifierChain(). AbstractPlatform::quoteIdentifierChain() hard-codes " with \" escaping and ignores quoteIdentifiers. phpdb-mysql and phpdb-pgsql override it. phpdb-sqlite does not.
• MySQL fragment pattern. phpdb-mysql's quoteIdentifierFragmentPattern already treats - as a word character, so x-y rendered correctly there before this change. Spaces and quotes did not.
• Platform drivers. None of phpdb-mysql, phpdb-sqlite or phpdb-pgsql overrides processSet() or processInsert(). mysqli (?) and pgsql ($N) ignore the parameter name in the SQL; only the ParameterContainer key changes. Their PDO drivers keep c_N.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!