-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: Add type headers to response formats. #11914
Conversation
This allows clients to interpret the results of SQL queries without having to guess types.
docs/querying/sql.md
Outdated
@@ -879,6 +879,8 @@ Submit your query as the value of a "query" field in the JSON object within the | |||
|`query`|SQL query string.| none (required)| | |||
|`resultFormat`|Format of query results. See [Responses](#responses) for details.|`"object"`| | |||
|`header`|Whether or not to include a header row for the query result. See [Responses](#responses) for details.|`false`| | |||
|`typesHeader`|Whether or not to include type information in the header. Only applicable when `header` is `true`. See [Responses](#responses) for details.|`false`| | |||
|`sqlTypesHeader`|Whether or not to include type information in the header. Only applicable when `header` is `true`. See [Responses](#responses) for details.|`false`| |
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.
|`sqlTypesHeader`|Whether or not to include type information in the header. Only applicable when `header` is `true`. See [Responses](#responses) for details.|`false`| | |
|`sqlTypesHeader`|Whether or not to include SQL type information in the header. Only applicable when `header` is `true`. See [Responses](#responses) for details.|`false`| |
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.
Whoops, missed that. Fixed.
@@ -93,4 +96,44 @@ public void close() throws IOException | |||
{ | |||
jsonGenerator.close(); | |||
} | |||
|
|||
static void writeHeader( |
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.
Do we need to keep these new methods static?
If it's for testing, we could test these by calling the writeHeader
method provided by the interface ResultFormat.Writer
.
Same comment in other writer implementations too.
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.
ObjectWriter.writeHeader is static because ObjectLinesWriter also uses it; same for ArrayWriter.writeHeader and ArrayLinesWriter.
docs/querying/sql.md
Outdated
`X-Druid-SQL-Header-Included: yes` if `header` was set to true and if the version of Druid you are talking to | ||
understands the `typesHeader` and `sqlTypesHeader` parameters. This happens even if you do not set `typesHeader` | ||
or `sqlTypesHeader`. |
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.
`X-Druid-SQL-Header-Included: yes` if `header` was set to true and if the version of Druid you are talking to | |
understands the `typesHeader` and `sqlTypesHeader` parameters. This happens even if you do not set `typesHeader` | |
or `sqlTypesHeader`. | |
`X-Druid-SQL-Header-Included: yes` if `header` was set to true and if the version of Druid the client is connected to | |
understands the `typesHeader` and `sqlTypesHeader` parameters. Note that this header is set irrespective of whether `typesHeader` | |
or `sqlTypesHeader` are set or not. |
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.
I adopted similar language. Thanks for the suggestion.
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.
LGTM
This allows clients to interpret the results of SQL queries without having to guess types.