Skip to content
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

post json instead of sql in /v1/statement/ #2604

Closed
youngsofun opened this issue Nov 2, 2021 · 7 comments · Fixed by #2688
Closed

post json instead of sql in /v1/statement/ #2604

youngsofun opened this issue Nov 2, 2021 · 7 comments · Fixed by #2688
Assignees
Labels
A-query Area: databend query community-take

Comments

@youngsofun
Copy link
Member

youngsofun commented Nov 2, 2021

Summary

the first version of http handler, post raw sql statement to /v1/statement/.
I think it is better to post json instead to carry more context info.

{
"sql": "select * from number(10)" 
}

reasons

  1. one place to organize all context infos.
  2. header and params are key-value, need other effort to embed complex info
  3. params and header have encode requirements and length limitation.

how

since /v1/statement/ is used in cli and playground. we need to discuss the way to do it.

  1. let /v1/statement/ accept json only
  2. add a new endpoint /v1/query, abandon /v1/statement/
  3. add a new endpoint /v1/query for main use, keep /v1/statement/ for simple cases.

possible reason to reserve the raw_sql api:

  1. make it easy to write for tools like curl.
  2. it looks like the presto protocol (but it is difficult to be really compatible)
  3. click house use it to insert data (but we will have other api for data import)

sorry for not thinking it clearly to begin with.

@flaneur2020 @ZhiHanZ

related issue

#2241

@youngsofun
Copy link
Member Author

trino v2 plan to use json too

https://github.com/trinodb/trino/wiki/Trino-v2-client-protocol

@flaneur2020
Copy link
Member

the trino v2 protocol is much better IMO 🤔

@ZhiHanZ ZhiHanZ added A-query Area: databend query C-http-api labels Nov 2, 2021
@ZhiHanZ
Copy link
Collaborator

ZhiHanZ commented Nov 2, 2021

I prefer json schema. post json data provided advanced security guarantees and would be good to integrate needed fields without missing, on contrary, add a lot of queries in get would be quite messy

@youngsofun
Copy link
Member Author

@youngsofun
Copy link
Member Author

youngsofun commented Nov 2, 2021

@ZhiHanZ which do you prefer

  1. change /v1/statement/ directly to accept json only
  2. add a new endpoint /v1/query, abandon /v1/statement/
  3. add a new endpoint /v1/query for main use, keep /v1/statement/ for simple cases.

@ZhiHanZ
Copy link
Collaborator

ZhiHanZ commented Nov 2, 2021

@ZhiHanZ which do you prefer

  1. change /v1/statement/ directly to accept json only
  2. add a new endpoint /v1/query, abandon /v1/statement/
  3. add a new endpoint /v1/query for main use, keep /v1/statement/ for simple cases.

case 3 looks make sense, v1/statement is for debugging and simple demo, v1/query works for cloud platform

@youngsofun
Copy link
Member Author

/assignme

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query Area: databend query community-take
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants