cloud-fan commented on a change in pull request #35534:
URL: https://github.com/apache/spark/pull/35534#discussion_r809671431



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
##########
@@ -352,34 +352,44 @@ trait Unevaluable extends Expression {
  * An expression that gets replaced at runtime (currently by the optimizer) 
into a different
  * expression for evaluation. This is mainly used to provide compatibility 
with other databases.
  * For example, we use this to support "nvl" by replacing it with "coalesce".
- *
- * A RuntimeReplaceable should have the original parameters along with a 
"child" expression in the
- * case class constructor, and define a normal constructor that accepts only 
the original
- * parameters. For an example, see [[Nvl]]. To make sure the explain plan and 
expression SQL
- * works correctly, the implementation should also override flatArguments 
method and sql method.
  */
-trait RuntimeReplaceable extends UnaryExpression with Unevaluable {
-  override def nullable: Boolean = child.nullable
-  override def dataType: DataType = child.dataType
+trait RuntimeReplaceable extends Expression {
+  def replacement: Expression
+
+  override val nodePatterns: Seq[TreePattern] = Seq(RUNTIME_REPLACEABLE)
+  override def nullable: Boolean = replacement.nullable
+  override def dataType: DataType = replacement.dataType
   // As this expression gets replaced at optimization with its `child" 
expression,
   // two `RuntimeReplaceable` are considered to be semantically equal if their 
"child" expressions
   // are semantically equal.
-  override lazy val preCanonicalized: Expression = child.preCanonicalized
-
-  /**
-   * Only used to generate SQL representation of this expression.
-   *
-   * Implementations should override this with original parameters
-   */
-  def exprsReplaced: Seq[Expression]
-
-  override def sql: String = mkString(exprsReplaced.map(_.sql))
+  override lazy val preCanonicalized: Expression = replacement.preCanonicalized
 
-  final override val nodePatterns: Seq[TreePattern] = Seq(RUNTIME_REPLACEABLE)
+  final override def eval(input: InternalRow = null): Any =
+    throw QueryExecutionErrors.cannotEvaluateExpressionError(this)
+  final override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): 
ExprCode =
+    throw QueryExecutionErrors.cannotGenerateCodeForExpressionError(this)
+}
 
-  def mkString(childrenString: Seq[String]): String = {
-    prettyName + childrenString.mkString("(", ", ", ")")
+/**
+ * A special variant of [[RuntimeReplaceable]]. It makes `replacement` the 
child of the expression,
+ * to inherit the type coercion rules for it. The implementation should put 
`replacement` in the
+ * case class constructor, and define a normal constructor that accepts only 
the original
+ * parameters, or use `ExpressionBuilder`. For an example, see [[TryAdd]]. To 
make sure the explain
+ * plan and expression SQL works correctly, the implementation should also 
implement the
+ * `actualInputs` method, which should extracts the actual inputs after type 
coercion from
+ * `replacement`.
+ */
+trait RuntimeReplaceableInheritingTypeCoercion

Review comment:
       We need it. `inputTypes` doesn't work for `TryAdd`, it needs more 
complicated type coercion.




-- 
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]

Reply via email to