Quote Select, join and GROUP BY column names as identifiers

[simon-mundy]

Problem

Select fragment-quotes column names that are not plain or dotted words. On the SQL-92 platform:

Column string	Rendered before	Rendered now
x-y	"foo"."x""-""y" AS "x-y"	"foo"."x-y" AS "x-y"
my col	"foo"."my" "col" AS "my col"	"foo"."my col" AS "my col"
a"b	"foo"."a""'""b" AS "a'b"	"foo"."a'b" AS "a'b"

The alias was already quoted as a whole name, so the column and its alias disagreed. Join column lists and GROUP BY render the same broken way.

Cause

processSelect() (select and join columns) and processGroup() resolve each column string through AbstractSql::resolveColumnValue() with isIdentifier. That ends in AbstractPlatform::quoteIdentifierInFragment(), which splits the string on every character outside the platform's word pattern and quotes each piece. Only *, space, . and as are left bare (SAFE_WORDS). Update SET columns had the same cause, fixed in #238.

Rule

Select now overrides resolveColumnValue() for an identifier reference whose column is a string. Whole-name quoting is used only where fragment quoting renders broken SQL today:

1. Inline alias. A string containing as (case-insensitive, spaces on both sides) keeps fragment quoting, as before. processSelect() already detects this form with stripos($columnName, ' as ') so it doesn't add a second alias.
2. Valid fragment. The string is split on the delimiters that fragment quoting leaves bare: space, . and *. If fragment quoting renders it as valid SQL, it keeps fragment quoting, so the output is byte-identical to today. That holds when both of these are true:
   • every name piece between those delimiters is quoted whole by the platform, i.e. quoteIdentifierInFragment($piece) === quoteIdentifier($piece), so the fragment split never produces "x""-""y";
   • no two name pieces are separated by spaces alone, which would produce "my" "col".
3. Otherwise the string is a broken fragment and is quoted as an identifier chain. It is split only on ., each part is quoted with quoteIdentifier(), and the parts are joined with getIdentifierSeparator(). A * part stays bare.

Step 2 asks the platform itself, so it follows each platform's own fragment pattern. For example, MySQL treats - as a word character, so x-y keeps its existing `x-y` there. With quoteIdentifiers off, both quoting methods return the string as given, so every string keeps the fragment path and output does not change.

Expressions, sub-selects and every other use of resolveColumnValue() (Update, Insert) still go through the parent method unchanged.

What stays the same

docs/book, the unit tests, TableGateway/RowGateway and phpdb-mysql, phpdb-sqlite and phpdb-pgsql only pass these forms as column strings:

• plain names (bar)
• dotted names (t.bar)
• *
• aliased keys (['alias' => 'bar'])

SQL in a column always goes through Expression. All of these render byte-for-byte as before. So does every string that renders as valid SQL today, including:

• t.* ("t".*)
• arithmetic such as price * qty ("price" * "qty"), a*b ("a"*"b") and t.price * t.qty ("t"."price" * "t"."qty")
• the inline alias bar AS baz ("bar" AS "baz")

Behaviour change

This change is not breaking: output only changes for strings whose output was invalid SQL. Those are strings where the fragment split glued quoted pieces together or quoted a delimiter that isn't a safe word:

• hyphen: x-y
• quote character: a"b
• space-separated name: my col
• function call: COUNT(id), which was "COUNT""(""id"")" and is now "COUNT(id)". It is still not a function call.

SQL in a column list should still use new Expression(...), as the docs show.

Tests

test/unit/Sql/SelectColumnNameTest.php (#[Group('unit')]) has three data-provider tests, one each for select columns, join columns and GROUP BY. Cases:

• names: plain, dotted, *, t.*, hyphen, space, quote character, dotted hyphen or space
• arithmetic: with and without spaces, aliased, dotted, and with a hyphenated name
• other loose SQL: inline AS (upper and lower case), function call
• Expression columns
• each where relevant aliased and unaliased, and with and without the table prefix

Run against the 0.6.x Select.php, only the cases broken today fail: hyphen, space, quote character, function call, and arithmetic with a hyphenated name. Every other case, including all the plain arithmetic cases, passes unchanged. The changed lines in Select.php are fully covered.

• ./vendor/bin/phpunit --testsuite "unit test": OK (2237 tests)
• mago format --check, mago lint, mago analyze: all exit 0

Follow-ups (not in this PR)

• ORDER BY: processOrder() passes the parsed key straight to quoteIdentifierInFragment(), so x-y DESC renders "x""-""y" DESC. The cause is the same, but the order string is a small parsed language ('a DESC, b'), so it should be a separate change.
• Qualified star alias: an unaliased t.* in the select list or a join list still gets AS "t.*" appended ("t".* AS "t.*"), which is invalid SQL. A bare * is already exempt.
• Inline AS in join columns and GROUP BY: the join list appends a second alias ("zac"."bar" AS "baz" AS "bar AS baz"), unlike the select list.
• Platform quoting (from Quote Update SET columns as identifiers and bind them under valid placeholder names #238): SQL-92 and SQLite use ' for quoteIdentifierTo, so an embedded " becomes ' rather than "".

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!