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

Hequn Cheng commented on FLINK-12298:
-------------------------------------

[~dawidwys] Hi, I think it's good to make our API type safe. The proposal is 
good!

However, I also think it's better to introduce this when we design the Java 
DSL. For Java users, the select operation can only select a string now, i.e., 
select("a, withColumns(idx(1).to(10))"), this gives no typesafe guarantee even 
we introduce the typesafe range. 

I think this is also what jincheng means. We totally agree the typesafe is good 
and it's valuable to introduce the range. The only concern we have is we think 
it's useful when we have the Java DSL.

Best, Hequn

> 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