ClickHouse / clickhouse-java

ClickHouse Java Clients & JDBC Driver
https://clickhouse.com
Apache License 2.0
1.45k stars 534 forks source link

ClickHouseSqlParser fails to parse ternary operator in select #892

Open debychkov opened 2 years ago

debychkov commented 2 years ago

Minimal example:

2022.04.05 21:26:58,791 [main] WARN  [] com.clickhouse.jdbc.parser.ClickHouseSqlParser - Parse error at line 1, column 15.  Encountered: :2. If you believe the SQL is valid, please feel free to open an issue on Github with this warning and the following SQL attached.
SELECT 1=1?2=2:2=3 AS bool

Clickhouse command line client accepts this query:

my.ch.host :) SELECT 1=1?2=2:3=2

SELECT if(1 = 1, 2 = 2, 3 = 2)

Query id: d0196789-dbb5-4edf-86bf-be0e02c995a2

┌─if(equals(1, 1), equals(2, 2), equals(3, 2))─┐
│                                            1 │
└──────────────────────────────────────────────┘

Note: I'm still on the legacy driver, but it seems unimportant - parser is same for both drivers. But there is something - I'm not sure, how it will behave with new driver, but in legacy it can still cause problems. Despite having fallback on sending unparsed sql "as is" and successful server query, it may lead to losing FORMAT clause, and thus exceptions when parsing server response.

zhicwu commented 2 years ago

Thanks @debychkov. You can use parenthesis as a workaround. The loose parser also problem dealing with case ... when as mentioned in #723. It's not worthy of implementing a fully functional parser on client side, so the current plan is to move the parser to clickhouse-jdbc-ext, which is a new module for extended functions over clickhouse-jdbc like SQL templating(which will simplify queries using JDBC table function). And use same approach at here for parsing.

Despite having fallback on sending unparsed sql "as is" and successful server query, it may lead to losing FORMAT clause, and thus exceptions when parsing server response.

You're right. But the issue is only limited to legacy driver. In new driver, X-ClickHouse-Format http header is used instead.