GroupBy array-based result rows. #8196
Conversation
Fixes apache#8118; see that proposal for details. Other than the GroupBy changes, the main other "interesting" classes are: - ResultRow: The array-based result type. - BaseQuery: T is no longer required to be Comparable. - QueryToolChest: Adds "decorateObjectMapper" to enable query-aware serialization and deserialization of result rows (necessary due to their positional nature). - QueryResource: Uses the new decoration functionality. - DirectDruidClient: Also uses the new decoration functionality. - QueryMaker (in Druid SQL): Modifications to read ResultRows. These classes weren't changed, but got some new javadocs: - BySegmentQueryRunner - FinalizeResultsQueryRunner - Query
| { | ||
| return new CacheStrategy<Row, Object, GroupByQuery>() | ||
| return new CacheStrategy<ResultRow, Object, GroupByQuery>() | ||
| { | ||
| private static final byte CACHE_STRATEGY_VERSION = 0x1; |
There was a problem hiding this comment.
Should this version be bumped so off-machine caches don't cause an issue or will the fancy decorated mapper be used in like CachingQueryRunner for reading the values from the cache?
There was a problem hiding this comment.
Check out the prepareForCache method -- the cache format is the same. It was always array-based, and it remains so.
There was a problem hiding this comment.
Oops, looking closer I see this now, thanks 👍
| @@ -163,7 +164,9 @@ public Response cancelQuery(@PathParam("id") String queryId, @Context final Http | |||
| public Response doPost( | |||
| final InputStream in, | |||
| @QueryParam("pretty") final String pretty, | |||
| @Context final HttpServletRequest req // used to get request content-type,Accept header, remote address and auth-related headers | |||
|
|
|||
| // used to get request content-type,Accept header, remote address and auth-related headers | |||
There was a problem hiding this comment.
nit: .. request content-type,Accept header, .. -> .. request content-type, Accept header, ..
(i know it was already like that, so imo only fix if you have something else to change).
There was a problem hiding this comment.
I'll update this if there ends up being something else to change, for now I'll leave it as it was before so CI doesn't get interrupted.
| { | ||
| return new CacheStrategy<Row, Object, GroupByQuery>() | ||
| return new CacheStrategy<ResultRow, Object, GroupByQuery>() | ||
| { | ||
| private static final byte CACHE_STRATEGY_VERSION = 0x1; |
There was a problem hiding this comment.
Oops, looking closer I see this now, thanks 👍
Fixes #8118; see that proposal for additional details about the main change.
Other than the GroupBy changes, the main other "interesting" new/changed classes are:
and deserialization of result rows (necessary due to their positional nature).
These classes weren't changed, but got some new javadocs: