FrankChen021 commented on PR #13022:
URL: https://github.com/apache/druid/pull/13022#issuecomment-1242876576
> Here's a follow-on idea. Modifying a context map can perhaps be left to
working directly with the map, since it is mostly just calling `put()` along
with a few other items. So, perhaps we can just focus on reading the context.
Maybe we define:
>
> ```java
> interface Query<...> {
> default QueryContext getQueryContext() { return new
QueryContext(getContext()); }
> ```
>
> Then, `QueryContext` has all the many accessor methods mentioned above.
That is, `QueryContext` is a transient facade on top of the map.
>
> With this approach, all the myriad `getFoo` and `parseFoo` methods in
`QueryContexts` can move to `QueryContext`. In this approach, the goal of this
PR (type safety) would be provided by the typed `get{Type}` methods on
`QueryContext`.
>
> What do you think? If this will work, I can perhaps modify the PR I
mentioned to include this approach.
Hi @paul-rogers
When I touch this part in this PR, I noticed there are two classes,
`QueryContext` and `QueryContexts`, the latter one provides a group by getXXXX
helpers that QueryContext can be used. And also, another group of
`getContextXXXX` methods are defined in `Query` to get values from
`QueryContext`.
As you can seen in this PR, if a new `getXXX` method is added, we have to
add it in 3 classes to follow current pattern. These are too complicated. I
didn't make changes to this part because it involves a huge refactoring.
Basically, I agree with you that there should be a facade `QueryContext`
there(but I think QueryContext should not expose `Map` object to callers
directly), and a series of `getXXXX` methods are defined in `QueryContext` to
get values. And at last those helper motheds in `Query` are removed or
deprecated.
My focus in this PR is on the safe type conversion at caller side. And the
work has been done. How about let's merge this first and then resolve the
conflicts after you stablize the core changes on your branch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
