JingsongLi commented on a change in pull request #8435:
[FLINK-12443][table-planner-blink] Replace InternalType with LogicalType in
blink
URL: https://github.com/apache/flink/pull/8435#discussion_r283687864
##########
File path:
flink-table/flink-table-planner-blink/src/main/java/org/apache/flink/table/functions/aggfunctions/RankLikeAggFunctionBase.java
##########
@@ -98,30 +108,30 @@ protected Expression orderKeyEqualsExpression() {
return ret.orElseGet(() -> literal(true));
}
- protected Expression generateInitLiteral(InternalType orderType) {
- if (orderType.equals(InternalTypes.BOOLEAN)) {
+ protected Expression generateInitLiteral(LogicalType orderType) {
+ if (orderType instanceof BooleanType) {
Review comment:
For simple codes or codes in Java, I prefer use the logical type root.
But for complex type codes in Scala, I prefer use type to check directly.
Just like:
```
t match {
case rowType: RowType => // use rowType do some thing
case arrayType: ArrayType => // use arrayType do some thing
}
```
I think it is better than:
```
t.getTypeRoot match {
case LogicalTypeRoot.ROW => t.asInstanceOf[RowType] // use rowType do some
thing
case LogicalTypeRoot.ARRAY => t.asInstanceOf[ArrayType] // use rowType do
some thing
}
```
What do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
