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