Quote ORDER BY names as identifiers where fragment quoting breaks them

[simon-mundy]

Problem

Select::order() renders some order names as broken SQL. On the SQL-92 platform:

Order	Rendered before	Rendered now
'x-y DESC'	"x""-""y" DESC	"x-y" DESC
't-1.x-y DESC'	"t""-""1"."x""-""y" DESC	"t-1"."x-y" DESC
['my col' => 'DESC']	"my" "col" DESC	"my col" DESC
'a"b DESC'	"a""'""b" DESC	"a'b" DESC
'café DESC'	each non-ASCII byte quoted as its own piece	"café" DESC

The same happens in comma lists ('x-y DESC, b') and in list arrays (['x-y DESC', 'b']).

Cause

processOrder() passes each parsed order name to AbstractPlatform::quoteIdentifierInFragment(). That regex-splits on every non-word character and quotes each piece, leaving only the safe words (*, space, ., as) bare. Select columns, join columns and GROUP BY have the same cause (#241), as do Update SET columns (#238). #241 lists ORDER BY as a follow-up.

Rule

processOrder() now calls a new private Select::quoteOrderIdentifier():

1. The name is cut into segments at every * and ., and spaces around each segment are trimmed.
2. If every segment renders through quoteIdentifierInFragment() either as one quoted identifier (the same as quoteIdentifier()) or unchanged (empty, a bare safe word, or identifier quoting off), the name keeps fragment quoting as before.
3. Otherwise the name is quoted whole as an identifier chain: it is split only on ., each part goes through quoteIdentifier(), a * part stays bare, and the parts are joined with getIdentifierSeparator().

The check uses the platform's own quoteIdentifierInFragment(), so platform fragment patterns are respected. phpdb-mysql treats - as a word character, so x-y and price*x-y already rendered as valid SQL there and still render the same.

The order-string parsing is unchanged: comma splitting, the first-space split into name and direction, ASC/DESC detection and Expression entries work as before.

What stays the same

Any order whose fragment output was valid renders byte-for-byte as before. The tests pin these forms:

• plain and dotted names: a, t.col DESC
• comma lists: a DESC, b
• arithmetic: price*qty DESC, ['price * qty' => 'DESC'], ['t.price*t.qty' => 'DESC']
• 'my col DESC', which still parses as the name my with an unknown direction ("my" ASC)
• a trailing space in an array key, a bare safe-word part such as t.as, and unknown directions
• Expression entries, alone and in arrays
• identifier quoting off, where both paths return the name as given

Only output that was already broken changes. The maintainer's long-term direction (D18) is that identifier slots take identifiers only and SQL goes through Expression. On 0.6.x, this PR changes only broken output.

Tests

test/unit/Sql/SelectOrderNameTest.php (#[Group('unit')]) has three data-provider tests:

• broken forms now quoted as identifiers (14 cases): hyphen, quote character, non-ASCII, dotted hyphen, a hyphen next to *, and x-y.*, across the string, comma-list, array-key and list-array forms, with ASC and DESC
• valid forms rendered unchanged (16 cases), listed above
• platform rules (5 cases): a new HyphenWordSql92Platform test asset uses phpdb-mysql's fragment pattern, and TestSql92Platform covers identifier quoting off

Before the fix, all 14 broken-form cases and the x+y case on the hyphen-word platform failed, and every unchanged case passed. The changed lines in Select.php are fully covered.

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!