metanil commented on code in PR #55885:
URL: https://github.com/apache/spark/pull/55885#discussion_r4148029971
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/TransformExpression.scala:
##########
@@ -52,23 +87,46 @@ case class TransformFunctionId(canonicalName: String,
numBucketsOpt: Option[Int]
case class TransformExpression(
function: BoundFunction,
children: Seq[Expression],
- numBucketsOpt: Option[Int] = None,
- reducedWith: Option[TransformFunctionId] = None) extends Expression {
+ reducedWith: Option[TransformFunctionId] = None) extends Expression with
Logging {
override def nullable: Boolean = true
/** Drops the trailing `reducedWith` when it is unset, so the ordinary form
is unchanged. */
override protected def stringArgs: Iterator[Any] =
if (reducedWith.isEmpty) super.stringArgs.take(productArity - 1) else
super.stringArgs
- /** The identity of this expression's transform function. */
- lazy val functionId: TransformFunctionId =
- TransformFunctionId(function.canonicalName(), numBucketsOpt)
+ /**
+ * Extract literal children (constant parameters) from this transform. These
are constant
+ * arguments like the bucket count in `bucket(n, col)` or the width in
`truncate(col, width)`.
+ * Positions are dropped, so this is for handing parameters to a connector
reducer, NOT for
+ * identity -- see [[functionId]] and [[TransformFunctionId.argumentShapes]].
+ */
+ private lazy val literalChildren: Seq[Literal] =
+ children.collect { case l: Literal => l }
/**
- * Whether this [[TransformExpression]] has the same semantics as `other`.
- * For instance, `bucket(32, c)` is equal to `bucket(32, d)`, but not to
`bucket(16, d)` or
- * `year(c)`.
+ * This transform's identity, or None if some argument is not a literal, a
nested transform or a
+ * column reference (e.g. `c + 1`); such a transform is not the same as any
other.
+ */
+ lazy val functionId: Option[TransformFunctionId] = {
+ import TransformFunctionId.ArgumentShape
+ val shapes = children.map {
+ case l: Literal => Some(ArgumentShape.Param(l))
+ case t: TransformExpression => t.functionId.map(ArgumentShape.Nested(_))
+ case c if TransformExpression.isColumnRef(c) =>
Some(ArgumentShape.Column)
Review Comment:
Thanks @peter-toth for the update. I was exploring both findings (1 and 4)
yesterday. I'll for wait for your PR #59165 and rebase it here. I'll try to
preemptively pull it before the merge to see how it fits here, and be ready.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
