Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6942660361
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| literalPos := p.Pos() | ||
| savedState := p.lexer.saveState() | ||
| _ = p.lexer.consumeToken() | ||
| if p.matchTokenKind(TokenKindString) { | ||
| return p.parseString(literalPos) |
There was a problem hiding this comment.
When a caller parses and formats a typed literal such as SELECT DATE '2024-01-01', this returns a plain StringLiteral after consuming the type keyword, so formatting produces SELECT '2024-01-01' (as the new golden file demonstrates). That changes the expression from a typed Date/Timestamp literal into a String and can alter comparisons, overload resolution, and result schemas; the AST must retain and re-emit the consumed literal type.
Useful? React with 👍 / 👎.
| modifiers := make([]*FunctionExpr, 0) | ||
| for { | ||
| if p.matchKeyword(KeywordExcept) || p.matchKeyword(KeywordApply) || p.matchKeyword(KeywordReplace) { | ||
| if (p.matchKeyword(KeywordExcept) && !p.peekKeyword(KeywordSelect)) || p.matchKeyword(KeywordApply) || p.matchKeyword(KeywordReplace) { |
There was a problem hiding this comment.
When the right operand is parenthesized, as in SELECT 1 EXCEPT (SELECT 2), the next token is ( rather than SELECT, so this branch consumes EXCEPT as a select-item modifier. The parser consequently leaves SelectQuery.Except nil and formats the statement as SELECT 1 EXCEPT(SELECT 2) instead of representing the set operation, even though parseSelectQuery explicitly supports parenthesized operands; the modifier/set-operation disambiguation must account for this form.
Useful? React with 👍 / 👎.
Fix DATE and TIMESTAMP literals by consuming the type keyword before parsing the following string. Disambiguate projection EXCEPT modifiers from EXCEPT set operations using SELECT lookahead, without changing the AST or global keyword rules. Before applying this PR: - SELECT DATE '2024-01-01' and SELECT TIMESTAMP '2024-01-01 00:00:00' failed because the type keyword was still current when the string was parsed. - SELECT 1 EXCEPT SELECT 2 failed because EXCEPT was treated as a projection modifier and expected modifier arguments. After this PR: - Typed date literals, keyword identifiers and aliases, projection modifiers, and EXCEPT set operations parse in their production-specific contexts. - Invalid forms such as SELECT * EXCEPT FROM t and SELECT 1 EXCEPT 2 remain rejected. Add focused parser tests and golden fixtures for the supported forms. Verification: make test; pinned golangci-lint v1.53.3; ClickHouse local syntax checks for positive and negative cases.
6942660 to
4027b79
Compare
git-hulk
changed the title
1 participant
Fix DATE and TIMESTAMP literals by consuming the type keyword before parsing the following string. Disambiguate projection EXCEPT modifiers from EXCEPT set operations using SELECT lookahead, without changing the AST or global keyword rules.
Before applying this PR:
After this PR:
Add focused parser tests and golden fixtures for the supported forms.
Verification: make test; pinned golangci-lint v1.53.3; ClickHouse local syntax checks for positive and negative cases.