Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-25044][SQL][followup] add back UserDefinedFunction.inputTypes #22319

Closed
wants to merge 9 commits into from
Closed

[SPARK-25044][SQL][followup] add back UserDefinedFunction.inputTypes #22319

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e9c6fbc
add back UserDefinedFunction.inputTypes
cloud-fan Sep 3, 2018
5acb1df
add back unapply
cloud-fan Sep 3, 2018
2245395
fix
cloud-fan Sep 3, 2018
01a51ca
fix
cloud-fan Sep 3, 2018
c77db87
another fix
cloud-fan Sep 3, 2018
d290fec
fix apply conflict
cloud-fan Sep 3, 2018
4791240
fix a mistake
cloud-fan Sep 4, 2018
2b2c894
revert mima
cloud-fan Sep 4, 2018
9e060a4
another fix
cloud-fan Sep 5, 2018
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
20 changes: 17 additions & 3 deletions sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,18 @@ import org.apache.spark.sql.types.DataType
case class UserDefinedFunction protected[sql] (
f: AnyRef,
dataType: DataType,
inputTypes: Option[Seq[ScalaReflection.Schema]]) {
inputSchemas: Option[Seq[ScalaReflection.Schema]]) {

private var _nameOption: Option[String] = None
private var _nullable: Boolean = true
private var _deterministic: Boolean = true

// This is to keep backward compatibility for this case class.
// TODO: revisit this case class in Spark 3.0, and narrow down the public surface.
def inputTypes: Option[Seq[DataType]] = {
Copy link
Member

@HyukjinKwon HyukjinKwon Sep 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@cloud-fan, I think this still breaks compatibility when UserDefinedFunction's used in a pattern match.

inputSchemas.map(_.map(_.dataType))
}

/**
* Returns true when the UDF can return a nullable value.
*
Expand All @@ -73,11 +79,11 @@ case class UserDefinedFunction protected[sql] (
f,
dataType,
exprs.map(_.expr),
inputTypes.map(_.map(_.dataType)).getOrElse(Nil),
inputSchemas.map(_.map(_.dataType)).getOrElse(Nil),
udfName = _nameOption,
nullable = _nullable,
udfDeterministic = _deterministic,
nullableTypes = inputTypes.map(_.map(_.nullable)).getOrElse(Nil)))
nullableTypes = inputSchemas.map(_.map(_.nullable)).getOrElse(Nil)))
}

private def copyAll(): UserDefinedFunction = {
Expand Down Expand Up @@ -129,3 +135,11 @@ case class UserDefinedFunction protected[sql] (
}
}
}

object UserDefinedFunction {
Copy link
Member

Choose a reason for hiding this comment

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

private[sql] since we don't explicitly mention expressions package is meant to be internal.

Copy link
Member

Choose a reason for hiding this comment

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

I am okay if this is the only way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I'm adding back the public unapply method, so this must be public.

// This is to keep backward compatibility for this case class.
// TODO: revisit this case class in Spark 3.0, and narrow down the public surface.
def unapply(arg: UserDefinedFunction): Option[(AnyRef, DataType, Option[Seq[DataType]])] = {
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this still break binary compatibility since we bind to another signature?

Copy link
Member

Choose a reason for hiding this comment

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

I checked locally and apparently not - looks explicit unapply here and unapply from case class are handled as the same signature from my cursory test.

Some(arg.f, arg.dataType, arg.inputSchemas.map(_.map(_.dataType)))
}
}