Repository navigation
Quote ORDER BY names as identifiers where fragment quoting breaks them - #243
Open
simon-mundy wants to merge 12 commits into
Open
simon-mundy wants to merge 12 commits into
simon-mundy wants to merge 12 commits into
Conversation
Select resolved each column string through quoteIdentifierInFragment(), which splits on every non-word character, so a column such as x-y rendered as "x""-""y" while its alias rendered as "x-y". Column strings in the select list, join column lists and GROUP BY are now quoted as an identifier chain split only on the table-qualifier dot, with a * part left bare. A string holding an inline " AS " alias keeps fragment quoting, as before.
A column string now switches to identifier-chain quoting only when fragment quoting would render it broken: a piece between the safe delimiters (space, "." and "*") that the platform would split and quote in parts, as with x-y, a"b or COUNT(id), or two names separated by spaces alone, as with "my col". Strings such as "price * qty" or "a*b" render exactly as before.
Select::processOrder() fragment-quoted each order name, so names such as x-y, my col, a"b or café rendered as several quoted pieces. Such names are now quoted whole as an identifier chain split only on '.'. Names whose parts between '*' and '.' already render as one identifier each, such as t.col and price*qty, render byte-for-byte as before.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Replace Select's own column-quoting helpers with IdentifierQuoter, which the ORDER BY and predicate fixes can share: - A plain name of word characters and qualifier dots takes a fast path straight to fragment quoting, as before. - Otherwise IdentifierShape reads the name in pieces. Fragment quoting is kept where it renders as written; a name it would split (x-y, my col, t.as) is quoted whole per qualifier part, with each part trimmed. - Expression syntax that no identifier quoting renders as written, such as COUNT(id), a - b or x-y * 2, throws InvalidArgumentException, as does an empty name or qualifier part. - An inline column alias such as 'x-y as z' renders "x-y" AS "z". TrustingHyphenWordPlatform becomes the shared hyphen-as-word test platform.
Replace quoteOrderIdentifier() with IdentifierQuoter::quote(), so ORDER BY names follow the same rule as columns: "t"."as" rather than the invalid "t".as, and arithmetic mixed with a hyphenated name, such as price*x-y, now throws instead of becoming one identifier. The tests use the shared TrustingHyphenWordPlatform, replacing HyphenWordSql92Platform.
# Conflicts: # src/Sql/Select.php
This was referenced Nov 4, 2026
The test restricts coverage to the factories it lists, so the new forEmptyIdentifierPart() and forUnquotableIdentifier() counted as uncovered and failed codecov/patch.
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.
Problem
Select::order()renders some order names as broken SQL. On the SQL-92 platform:'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'"café" DESC['t.as' => 'DESC']"t".as DESC"t"."as" DESCThe same happens in comma lists (
'x-y DESC, b') and in list arrays (['x-y DESC', 'b']).Cause
processOrder()passes each parsed order name toAbstractPlatform::quoteIdentifierInFragment(). That splits the name on every non-word character and quotes each piece, leaving only the safe words (*, space,.,as) bare. Select columns (#241), predicate names (#244) and Update SET columns (#238) have the same cause.Fix
processOrder()now quotes each name withIdentifierQuoter::quote(), the shared rule added in #241:price*qty, keep fragment quoting.price*x-yorCOUNT(id), throwsInvalidArgumentException, as does an empty name or qualifier part.The check asks the platform itself. phpdb-mysql treats
-as a word character, so`x-y`and`price`*`x-y`render as before there.The order-string parsing is unchanged: comma splitting, the first-space split into name and direction, ASC/DESC detection and
Expressionentries work as before.What stays the same
Every order whose output was valid renders byte-for-byte as before. The tests pin these forms:
a,t.col DESC;a DESC, b;price*qty DESC,['price * qty' => 'DESC'],['t.price*t.qty' => 'DESC'];'my col DESC', which still parses as the namemy(see the follow-up in Quote Select, join and GROUP BY column names as identifiers #241);Expressionentries, alone and in arrays;Tests
test/unit/Sql/SelectOrderNameTest.php(#[Group('unit')]) has four data-provider tests:x-y.*andt.as;TrustingHyphenWordPlatform(backtick quoting,-as a word character) and with identifier quoting off;price*x-y,COUNT(id), andx+yon the hyphen platform.The duplicate
HyphenWordSql92Platformtest asset is removed in favour of #241'sTrustingHyphenWordPlatform.mago format --check,mago lintandmago analyzeexit 0 on PHP 8.3, 8.4 and 8.5, and so doesmago guard.