-
Notifications
You must be signed in to change notification settings - Fork 752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
show query support format #6366
show query support format #6366
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Thanks for the contribution! Please review the labels and make any necessary changes. |
Need a stateless test for it, to avoid someone breaking it in the future, and we can easy to document it from the test case. |
Yes because the ClickHouse http handler will use old parser to parse the sql. If err it’ll directly return err. So I only add new parser test. And now in old parser not support format keyword in show or other query like desc. Maybe we can set the or to draft. when the planer v2 is enable default add some test? |
@@ -54,7 +54,7 @@ error: | |||
--> SQL:1:21 | |||
| | |||
1 | truncate table a.b.c.d | |||
| ^ expected `PURGE` or `;` | |||
| ^ expected `PURGE`, `FORMAT`, or `;` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/
Summary
Actually we have three ideas:
For every should support format cause's query modify the planner but that will modify so many planner code.
In clickhouse handler token the SQL and use nom cut the FORMAT keyword and then get the format value. But it has a bad case like this: select
format
from default.format
format xxx;In this pr, all of the statement is #statment_body and the format is always at the last of the statement body. So we just need to modify the final rule.
Changelog
Related Issues
Fixes #6298