Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix
  • Loading branch information
maropu committed Apr 19, 2018
commit 4bee661848f85144ec6672e1f6aefb6802b5cbe3
Original file line number Diff line number Diff line change
Expand Up @@ -1670,27 +1670,30 @@ case class ValidateExternalType(child: Expression, expected: DataType)

override def nullable: Boolean = child.nullable

override def dataType: DataType = RowEncoder.externalDataTypeForInput(expected)
override val dataType: DataType = RowEncoder.externalDataTypeForInput(expected)

private val errMsg = s" is not a valid external type for schema of ${expected.simpleString}"

private lazy val dataTypeClazz = if (dataType.isInstanceOf[ObjectType]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fold this into the checkType functions. I'd rather not deference a lazy val every time we check a type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I'll try

dataType.asInstanceOf[ObjectType].cls
} else {
// Some external types (e.g., native types and `PythonUserDefinedType`) might not be ObjectType
ScalaReflection.classForNativeTypeOf(dataType)
}

private lazy val checkType = expected match {
case _: DecimalType =>
(value: Any) => {
Seq(classOf[java.math.BigDecimal], classOf[scala.math.BigDecimal], classOf[Decimal])
.exists { x => value.getClass.isAssignableFrom(x) }
value.isInstanceOf[java.math.BigDecimal] || value.isInstanceOf[scala.math.BigDecimal] ||
value.isInstanceOf[Decimal]
}
case _: ArrayType =>
(value: Any) => {
value.getClass.isAssignableFrom(classOf[Seq[_]]) || value.getClass.isArray
}
case _ if ScalaReflection.isNativeType(expected) =>
(value: Any) => {
value.getClass.isAssignableFrom(ScalaReflection.classForNativeTypeOf(expected))
value.getClass.isArray || value.isInstanceOf[Seq[_]]
}
case _ =>
(value: Any) => {
value.getClass.isAssignableFrom(dataType.asInstanceOf[ObjectType].cls)
dataTypeClazz.isInstance(value)
}
}

Expand All @@ -1715,7 +1718,7 @@ case class ValidateExternalType(child: Expression, expected: DataType)
Seq(classOf[java.math.BigDecimal], classOf[scala.math.BigDecimal], classOf[Decimal])
.map(cls => s"$obj instanceof ${cls.getName}").mkString(" || ")
case _: ArrayType =>
s"$obj instanceof ${classOf[Seq[_]].getName} || $obj.getClass().isArray()"
s"$obj.getClass().isArray() || $obj instanceof ${classOf[Seq[_]].getName}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need to change codegen implementation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the @rednaxelafx 's comment, I changed this codegen, too. #20757 (comment)

case _ =>
s"$obj instanceof ${CodeGenerator.boxedType(dataType)}"
}
Expand Down