FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[bug] signed numeric literal after ')' or ']' is treated as a literal instead of a binary operator · Issue #286 · AfterShip/clickhouse-sql-parser · GitHub

[bug] signed numeric literal after ')' or ']' is treated as a literal instead of a binary operator #286

Description

Context

We use this parser at SigNoz to validate user-authored ClickHouse SQL before it reaches our telemetry store. This issue surfaced while running our production query corpus through the parser: the statements are valid ClickHouse but produce no AST, so they cannot be validated.

We would like to contribute the fix for this upstream rather than work around it locally, and we intend to do the same for the other gaps we find. Reporting first so the approach can be agreed before we open a PR.

Summary

-1 / +1 following a closing bracket is lexed as a single signed-literal token, so the parser sees two adjacent expressions with no operator between them and rejects SQL that ClickHouse accepts.

Reproduces on v0.5.2 and on master (e4c6e11).

Reproduction

_, err := parser.NewParser("SELECT (1)-1").ParseStmts()
line 1:11 <EOF> or ';' was expected, but got: "-1"
SELECT (1)-1
          ^^

All of the following are rejected:

SELECT (1)-1                                     -- <EOF> or ';' was expected, but got: "-1"
SELECT (1)+1                                     -- <EOF> or ';' was expected, but got: "+1"
SELECT (1)-1.5                                   -- <EOF> or ';' was expected, but got: "-1.5"
SELECT arr[1]-1                                  -- <EOF> or ';' was expected, but got: "-1"
SELECT (f()-1)                                   -- expected ')', but got '<int>'
SELECT (now()-1)*2                               -- expected ')', but got '<int>'
SELECT (toUnixTimestamp(now())-3600)*1000000000   -- expected ')', but got '<int>'

ClickHouse accepts these

clickhouse local -q "SELECT (1)-1"

Proposed fix

Lexer.hasPrecedenceToken disambiguates unary from binary +/-, but its set of tokens that can terminate an expression omits the closing brackets:

func (l *Lexer) hasPrecedenceToken(last *Token) bool {
        return last != nil && (last.Kind == TokenKindIdent ||
                last.Kind == TokenKindKeyword ||
                last.Kind == TokenKindInt ||
                last.Kind == TokenKindFloat ||
                last.Kind == TokenKindString)
  }

This explains the split above: a-1 and 1-1 parse because an identifier and a number are both in the set, while (1)-1 and arr[1]-1 do not. A ) or ] also terminates an expression, so a following - is a binary operator in exactly the same way.

  1. Adding TokenKindRParen and TokenKindRBracket to that set resolves every case.
  2. Adding more fixtures under parser/testdata/query/ covering the bracket, function-call and array-index forms.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


    Back | FazBrowse Home | New Git URL