[ 
https://issues.apache.org/jira/browse/CALCITE-4410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17237607#comment-17237607
 ] 

Will Yu edited comment on CALCITE-4410 at 11/23/20, 6:48 PM:
-------------------------------------------------------------

[~julianhyde] 

??I guess we could add a {{SqlValidator getValidator()}} method to {{Planner}}; 
it would throw if you are at an early phase of planning and there is not yet a 
validator.??

Like this idea. Only a small concern is that whether users will get confused 
about whether they should use {{planner.getValidator().validate(sqlNode)}} or 
{{planner.validate(sqlNode)}} But not sure whether this could be a real issue 
or not. If so, I suggest to only add this method in {{PlannerImpl}} to mitigate 
this confusion. What do you think?

 

??We could instead add a {{<T> unwrap(Class<T> clazz)}} method to {{Planner}} 
(i.e. make {{Planner}} implement {{Wrapper}}) and people could call 
{{planner.unwrap(SqlValidator.class)}}.??

I took a look at the {{Wrapper}} class. It seems to be used to convert types. 
However, {{Planner}} interface does not extends {{Validator}} interface. So it 
looks not quite fit for this use case.

 

[~zabetak]

Thanks for calling out that method. I also find that method. The reason why I 
did not use it is because the constructed type is {{SqlValidatorImpl}}, but not 
{{CalciteSqlValidator}} A little concern I have on adding another factory 
method is that it will also create duplication, and confusion which one to use. 
But again, not sure whether it could be a real issue or not.

 


was (Author: my7ym):
[~julianhyde] 

?? I guess we could add a {{SqlValidator getValidator()}} method to 
{{Planner}}; it would throw if you are at an early phase of planning and there 
is not yet a validator.??

Like this idea. Only a small concern is that whether users will get confused 
about whether they should use {{planner.getValidator().validate(sqlNode)}} or 
{{planner.validate(sqlNode)}} But not sure whether this could be a real issue 
or not. If so, I suggest to only add this method in {{PlannerImpl}} to mitigate 
this confusion. What do you think?

 

?? We could instead add a {{<T> unwrap(Class<T> clazz)}} method to {{Planner}} 
(i.e. make {{Planner}} implement {{Wrapper}}) and people could call 
{{planner.unwrap(SqlValidator.class)}}.??

I took a look at the {{Wrapper}} class. It seems to be used to convert types. 
However, {{Planner}} interface does not extends {{Validator}} interface. So it 
looks not quite fit for this use case.

 

[~zabetak]

Thanks for calling out that method. I also find that method. The reason why I 
did not use it is because the constructed type is {{SqlValidatorImpl}}, but not 
{{CalciteSqlValidator}} A little concern I have on adding another factory 
method is that it will also create duplication, and confusion which one to use. 
But again, not sure whether it could be a real issue or not.

 

> Expose the type of internal sql node in Planner
> -----------------------------------------------
>
>                 Key: CALCITE-4410
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4410
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>    Affects Versions: 1.26.0
>            Reporter: Will Yu
>            Assignee: Will Yu
>            Priority: Minor
>
> Currently in Planner interface, there is only possible to fetch the 
> *RelDataType* of the whole SqlNode through *validateAndGetType*, but really 
> hard to fetch the type of one component in the SqlNode, e.g. one of the 
> statement in the select list.
> Currently, we worked it around by duplicate some code in Calcite, which is 
> not a good practice. It would be helpful to directly expose the 
> *SqlValidator::getValidatedNodeType* through Planner.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to