SQL parsing into an AST fails on INTERVAL and SAMPLE statements

[eKrajnak]

What happened:
Clickhouse query which contains:

• "INTERVAL 1 MINUTE" in where clause
  or
• "FROM table SAMPLE 1/10"
  fails at SQL parsing into an AST when adhoc filter is applied

What you expected to happen:
Apply adhoc filter and perform query

How to reproduce it (as minimally and precisely as possible):

Apply any adhoc filter and run the query:

SELECT *
FROM data
SAMPLE 1/10
WHERE time > NOW() - INTERVAL 1 MINUTE

and watch javascript console log:

Unexpected int token: "1". Instead, I was expecting to see one of the following:

    - A "lparen" token
    - A "kw_cross" token
    - A "kw_left" token
    - A "kw_right" token
    - A "kw_full" token
    - A "comma" token
    - A "kw_inner" token
    - A "kw_where" token
    - A "kw_join" token
    - A "kw_group" token
    - A "kw_order" token
    - A "kw_offset" token
    - A "kw_for" token
    - A "kw_limit" token
    - A "kw_fetch" token
    - A "kw_union" token
    - A "semicolon" token

Environment:

• Grafana version: 11.1.4 (2355de00c6)
• Plugin version: 4.3.2
• OS Grafana is installed on: Rocky Linux release 8.10 (Green Obsidian)
• User OS & Browser: macOS 14.6.1 and Chrome 127.0.6533.120

[SpencerTorres]

The AST is notorious for having SQL syntax problems. I'll try to reproduce this locally and see if it can be patched.

@bossinc We might need to start exploring other options for the AST. I believe one of its main purposes is for extracting the table name (at least for adhoc filters). Maybe we can add a field to the SQL UI where people can manually write the table name. Or we can find a good Go module on the backend to handle it and use that to get an AST via HTTP request to the plugin.

[eKrajnak]

Thank you.
For INTERVAL 1 MINUTE I can use workaround "toIntervalMinute(1)" already now.
But for SAMPLE I have no solution. I took a fast look on AST. If I'm correct it's AST plugin for Postgresql and it knows "TABLESAMPLE" with the very similar meaning. Maybe it could be just renamed.

[SpencerTorres]

The AST plugin is pretty flexible actually, maybe we can just add the keyword somewhere. Thanks for mentioning that

[eKrajnak]

Any update about the issue?

[SpencerTorres]

No update yet, although I've been looking into alternatives for the AST parser since this is just one of many issues related to the current implementation

[aangelisc]

Hi @eKrajnak, @SpencerTorres, I wonder if the recently merged changes will resolve this issue?