fix(engine): stop ORDER BY splitting quoted column names at the dot (… - #58
Merged
Merged
Conversation
…P34)
`ORDER BY "name.common"` failed with the self-contradicting error
`Column 'name.common' not found. Did you mean 'name.common'?` on any CSV
whose header carries dotted names (data/countries.csv). The same column
resolved fine in the SELECT list, which made it look like an ORDER BY
parsing bug.
It is not parsing. The AST is correct: the parser yields
ColumnRef { name: "name.common", quote_style: DoubleQuotes,
table_prefix: None }, and a genuinely qualified `countries.region` yields
name: "region" with table_prefix: Some("countries"). The bug was in
resolution — apply_multi_order_by_with_context treated any dot as a table
qualifier and looked up only the part after the last dot (`common`), which
does not exist. The suggestion was computed from the full name, hence the
error naming the column it had just refused to find.
Try the literal column name first, and fall back to qualifier-stripping
only for an unquoted reference. A double-quoted identifier is one name,
dots included — that is what the quotes are for. The fallback stays for
unquoted dotted names that older parse paths can still produce.
Files P34 (fixed) and R11 (open): resolve_column_index is documented as
the canonical resolver "used by all SQL clauses ... to ensure consistent
alias resolution" and already handles this case correctly, but ORDER BY
never called it and hand-rolled a worse copy. Converging them changes
behaviour for unquoted dotted names (qualified-name match vs. suffix
strip), so that is left to its own change with a parity run rather than
ridden along on a bug fix.
Verified on the reported query: all 250 rows come back in exactly the
order a 4-key lexicographic sort gives. Also checked DESC, ordering on a
quoted dotted column not in the SELECT list, GROUP BY on one, multi-dot
names ("translations.ara.common"), and that a genuinely missing dotted
column still errors rather than silently sorting wrong.
Tests: 1187 passed, 0 failed. cargo fmt and clippy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BfD1B6fURb22qrCwxjhc2q
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
1 participant
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.
…P34)
ORDER BY "name.common"failed with the self-contradicting errorColumn 'name.common' not found. Did you mean 'name.common'?on any CSV whose header carries dotted names (data/countries.csv). The same column resolved fine in the SELECT list, which made it look like an ORDER BY parsing bug.It is not parsing. The AST is correct: the parser yields ColumnRef { name: "name.common", quote_style: DoubleQuotes, table_prefix: None }, and a genuinely qualified
countries.regionyields name: "region" with table_prefix: Some("countries"). The bug was in resolution — apply_multi_order_by_with_context treated any dot as a table qualifier and looked up only the part after the last dot (common), which does not exist. The suggestion was computed from the full name, hence the error naming the column it had just refused to find.Try the literal column name first, and fall back to qualifier-stripping only for an unquoted reference. A double-quoted identifier is one name, dots included — that is what the quotes are for. The fallback stays for unquoted dotted names that older parse paths can still produce.
Files P34 (fixed) and R11 (open): resolve_column_index is documented as the canonical resolver "used by all SQL clauses ... to ensure consistent alias resolution" and already handles this case correctly, but ORDER BY never called it and hand-rolled a worse copy. Converging them changes behaviour for unquoted dotted names (qualified-name match vs. suffix strip), so that is left to its own change with a parity run rather than ridden along on a bug fix.
Verified on the reported query: all 250 rows come back in exactly the order a 4-key lexicographic sort gives. Also checked DESC, ordering on a quoted dotted column not in the SELECT list, GROUP BY on one, multi-dot names ("translations.ara.common"), and that a genuinely missing dotted column still errors rather than silently sorting wrong.
Tests: 1187 passed, 0 failed. cargo fmt and clippy clean.
Claude-Session: https://claude.ai/code/session_01BfD1B6fURb22qrCwxjhc2q