[FLINK-9742] Expose Expression.resultType to public

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 1.5.0
• Fix Version/s: 1.6.0
• Component/s: Table SQL / API
• Labels: pull-request-available
• Assignee: Jungtaek Lim
• Reporter: Jungtaek Lim
• Created: 04/Jul/18 13:25
• Updated: 05/Jul/18 19:55
• Resolved: 05/Jul/18 19:55

Description

I have use case of TableSource which requires custom implementation of TimestampExtractor. To ensure new TimestampExtractor to cover more general use cases, accessing Expression.resultType is necessary, but its scope is now defined as package private for "org.apache.flink".

Below is the implementation of custom TimestampExtractor which leverages Expression.resultType, hence had to place it to org.apache.flink package (looks like a hack).

class IsoDateStringAwareExistingField(val field: String) extends TimestampExtractor {
  override def getArgumentFields: Array[String] = Array(field)

  override def validateArgumentFields(argumentFieldTypes: Array[TypeInformation[_]]): Unit = {
    val fieldType = argumentFieldTypes(0)

    fieldType match {
      case Types.LONG => // OK
      case Types.SQL_TIMESTAMP => // OK
      case Types.STRING => // OK
      case _: TypeInformation[_] =>
        throw ValidationException(
          s"Field '$field' must be of type Long or Timestamp or String but is of type $fieldType.")
    }
  }

  override def getExpression(fieldAccesses: Array[ResolvedFieldReference]): Expression = {
    val fieldAccess: Expression = fieldAccesses(0)

    fieldAccess.resultType match {
      case Types.LONG =>
        // access LONG field
        fieldAccess
      case Types.SQL_TIMESTAMP =>
        // cast timestamp to long
        Cast(fieldAccess, Types.LONG)
      case Types.STRING =>
        Cast(Cast(fieldAccess, SqlTimeTypeInfo.TIMESTAMP), Types.LONG)
    }
  }
}

It would be better to just make Expression.resultType public to cover other cases as well. (I'm not sure other methods would be also better to be public as well.)

Issue Links

• links to: GitHub Pull Request #6252