KurtYoung 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_r290565250
 
 

 ##########
 File path: 
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/calcite/FlinkTypeFactory.scala
 ##########
 @@ -41,87 +40,112 @@ import scala.collection.JavaConverters._
 import scala.collection.mutable
 
 /**
-  * Flink specific type factory that represents the interface between Flink's 
[[InternalType]]
+  * Flink specific type factory that represents the interface between Flink's 
[[LogicalType]]
   * and Calcite's [[RelDataType]].
   */
 class FlinkTypeFactory(typeSystem: RelDataTypeSystem) extends 
JavaTypeFactoryImpl(typeSystem) {
 
   // NOTE: for future data types it might be necessary to
   // override more methods of RelDataTypeFactoryImpl
 
-  private val seenTypes = mutable.HashMap[(InternalType, Boolean), 
RelDataType]()
+  private val seenTypes = mutable.HashMap[LogicalType, RelDataType]()
 
-  def createTypeFromInternalType(
-      tp: InternalType,
-      isNullable: Boolean): RelDataType = {
+  def createTypeFromLogicalType(t: LogicalType, isNullable: Boolean): 
RelDataType = {
 
 Review comment:
   Since `LogicalType` already contains nullability information, i suggest we 
drop this interface, to force all the caller pass nullable through 
`LogicalType`. 

----------------------------------------------------------------
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

Reply via email to