Skip to content

ClickHouse: Support unparenthesized IN right-hand side#2385

Open
dinmukhamedm wants to merge 1 commit into
apache:mainfrom
lmnr-ai:clickhouse/allow-parse-in-no-paren
Open

ClickHouse: Support unparenthesized IN right-hand side#2385
dinmukhamedm wants to merge 1 commit into
apache:mainfrom
lmnr-ai:clickhouse/allow-parse-in-no-paren

Conversation

@dinmukhamedm

Copy link
Copy Markdown

Fixes #2384

Note: The fix is a little broader than just the parameterized ClickHouse queries, e.g. WHERE id IN {ids: Array(UUID)}.

After opening the issue, I experimented a little more with ClickHouse, and found that it wraps in parenthesis anything on the RHS of IN, even for singular values, e.g.

SELECT * from users WHERE name in 'a';
-- becomes
SELECT * from users WHERE name in ('a');

Comment thread src/parser/mod.rs
Comment on lines +4395 to +4396
// ClickHouse accepts a bare expression as the IN RHS (e.g. `x IN 'a'` or
// a `{name:Type}` placeholder), wrapping it into a single-element list.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// ClickHouse accepts a bare expression as the IN RHS (e.g. `x IN 'a'` or
// a `{name:Type}` placeholder), wrapping it into a single-element list.

#[test]
fn parse_in_unparenthesized_placeholder() {
// ClickHouse `{name:Type}` query-parameter placeholder as the IN RHS, without parens.
match clickhouse().expr_parses_to("x IN {ids:Array(UInt64)}", "x IN ({ids: Array(UInt64)})") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets rewrite these tests to use all_dialects_where and verified_stmt, we can remove the test for dialects that donts support the feature

Comment on lines +1853 to +1858
Expr::InList { list, negated, .. } => {
assert!(!negated);
assert_eq!(list.len(), 1);
assert!(matches!(list[0], Expr::Dictionary(_)));
}
other => panic!("expected InList, got {other:?}"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably drop the ast assertions, I think the funcationality is simple enough to rely only on roundtrip tests

Comment thread src/dialect/mod.rs
Comment on lines +439 to +442
/// side of `IN`, without a parenthesized list — as in `x IN 'a'` or the
/// ClickHouse `{name:Type}` query-parameter placeholder `x IN {ids:Array(UInt64)}`.
/// The expression is wrapped into a single-element list, matching ClickHouse,
/// which reformats `x IN 'a'` to `x IN ('a')`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// side of `IN`, without a parenthesized list — as in `x IN 'a'` or the
/// ClickHouse `{name:Type}` query-parameter placeholder `x IN {ids:Array(UInt64)}`.
/// The expression is wrapped into a single-element list, matching ClickHouse,
/// which reformats `x IN 'a'` to `x IN ('a')`.
/// side of `IN`, without a parenthesized list — as in `x IN 'a'`.

Comment thread src/dialect/clickhouse.rs
true
}

fn supports_in_unparenthesized_expr(&self) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets add a link to the docs where the syntax comes from

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IN operator rejects a ClickHouse query-parameter placeholder as its right-hand side without parenthesis

2 participants