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

SQL: Support more result formats, add columns header. #6191

Merged
merged 8 commits into from
Aug 27, 2018

Conversation

gianm
Copy link
Contributor

@gianm gianm commented Aug 18, 2018

  • Add result formats for line-based JSON and CSV.
  • Add X-Druid-Column-Names, X-Druid-Column-Types headers with a list of all columns that
    the response will contain.
  • Add more comprehensive documentation on what callers should expect
    when making Druid SQL queries.

- Add result formats for line-based JSON and CSV.
- Add X-Druid-Sql-Columns header with a list of all columns that
the response will contain.
- Add more comprehensive documentation on what callers should expect
when making Druid SQL queries.
@fjy fjy added this to the 0.13.0 milestone Aug 18, 2018
@gianm
Copy link
Contributor Author

gianm commented Aug 26, 2018

I'm not a huge fan of the 'trailers' here, but from what I can see, our version of Jetty (9.4.10.v20180503) will use chunked encoding, with a well formed final chunk, even if we throw an exception in the middle of a StreamingOutput (despite https://bugs.eclipse.org/bugs/show_bug.cgi?id=424107 which claims this behavior is fixed). I verified this by writing a patch that throws an exception midstream, and confirming in wireshark that the final chunk is there. So that means it's impossible for clients to detect a truncated response without some kind of trailer in the body itself.

Does anyone have any better ideas?

@gianm
Copy link
Contributor Author

gianm commented Aug 26, 2018

I realized that a single blank line is a better trailer (it still lets you detect truncation, and is easier to ignore). I'll change this.

@fjy fjy merged commit 0172326 into apache:master Aug 27, 2018
@gianm gianm deleted the sql-formats branch August 27, 2018 05:03
gianm added a commit to implydata/druid-public that referenced this pull request Aug 27, 2018
* SQL: Support more result formats, add columns header.

- Add result formats for line-based JSON and CSV.
- Add X-Druid-Sql-Columns header with a list of all columns that
the response will contain.
- Add more comprehensive documentation on what callers should expect
when making Druid SQL queries.

* Fix some tests.

* Adjust tests.

* Adjust trailer, add types header.

* Fix trailers.
gianm added a commit to implydata/druid-public that referenced this pull request Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants