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

sunjincheng commented on FLINK-12298:
-------------------------------------

Hi Dawid,

Thanks for mentioning the typesafe problem!

I think the typesafe problem does not only exist in the column functions. All 
the TableAPIs with String and Expression as parameters have the same problem. 
For example: `select("a, udf(b), ...")`, and the auxiliary operation APIs of 
Window is also the same, the parameters are only String and Expression, such as 
`Over.partitionBy(Expression...)`. Users can also write query like 
`Over.partitionBy(1, 2) ` .

So this is a general problem in TableAPI, as Timo said, we need to be 
considered from a Java DSL perspective. For example, `withColumns()` is a 
function call, then it should be written as follows(from 
[here|https://docs.google.com/document/d/1r3bfR9R6q5Km0wXKcnhfig2XQ4aMiLG5h2MTx960Fg8/edit?usp=sharing]
 ),

Select(call(withColumns, field("a"), range(...)))

When introducing Java DSL, we will introduce more types to users such as field, 
call, range etc.

>From the points of my view, I suggest this change be done in conjunction with 
>the Java DSL discussion, which we can discuss here 
>https://docs.google.com/document/d/1r3bfR9R6q5Km0wXKcnhfig2XQ4aMiLG5h2MTx960Fg8/edit?usp=sharing

What do you think?

> Make column functions accept custom Range class rather than Expression
> ----------------------------------------------------------------------
>
>                 Key: FLINK-12298
>                 URL: https://issues.apache.org/jira/browse/FLINK-12298
>             Project: Flink
>          Issue Type: Improvement
>          Components: Table SQL / API
>            Reporter: Dawid Wysakowicz
>            Priority: Major
>
> I would suggest to rework the column functions to a more typesafe approach 
> with custom {{Range}} class. Right now {{withColumns}} accepts array of 
> Expressions. We have 
> {{org.apache.flink.table.api.scala.ImplicitExpressionOperations#to}} method, 
> but also we have implicit conversion from {{scala.Range}} to Expression 
> range. This already introduces ambiguity, as it is unclear what will be the 
> end product of expression like {{1 to 9}}. This approach defers the checking 
> of the types of expressions to expressions resolution phase.
> I would suggest to make 
> {{org.apache.flink.table.api.scala.withColumns#apply}} always accept e.g. 
> {{ColumnRange}} that could always accept only {{Integer}} or {{String}} 
> instead of Expressions. Such class could look like (this is just a very rough 
> outline):
> {code}
> class ColumnRange {
>  IndexRange idx(Integer idx);
>  ReferenceRange ref(String reference);
> }
> class IndexRange {
>   IndexRange to(Integer idx);
> }
> class ReferenceRange {
>   ReferenceRange to(String ref);
> }
> {code}
> We could also have implicit conversion from {{scala.Range}} to {{IndexRange}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to