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



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala
##########
@@ -515,3 +515,96 @@ case class StringToMap(text: Expression, pairDelim: 
Expression, keyValueDelim: E
 
   override def prettyName: String = "str_to_map"
 }
+
+/**
+ * Adds/replaces field in struct by name.
+ *
+ * @param children Seq(struct, name, val)
+ */
+// scalastyle:off line.size.limit
+@ExpressionDescription(
+  usage = "_FUNC_(struct, name, val) - Adds/replaces field in struct by name.",
+  examples = """
+    Examples:
+      > SELECT _FUNC_(NAMED_STRUCT("a", 1, "b", 2), "c", 3);
+       {"a":1,"b":2,"c":3}
+      > SELECT _FUNC_(NAMED_STRUCT("a", 1, "b", 2), "b", 3);
+       {"a":1,"b":3}
+      > SELECT _FUNC_(CAST(NULL AS struct<a:int,b:int>), "c", 3);
+       {"a":null,"b":null,"c":3}

Review comment:
       this is a bit counter-intuitive. Most SQL functions return null if the 
input is null. Do we have a valid real-world use case to justify this behavior?

##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala
##########
@@ -515,3 +515,96 @@ case class StringToMap(text: Expression, pairDelim: 
Expression, keyValueDelim: E
 
   override def prettyName: String = "str_to_map"
 }
+
+/**
+ * Adds/replaces field in struct by name.
+ *
+ * @param children Seq(struct, name, val)
+ */
+// scalastyle:off line.size.limit
+@ExpressionDescription(
+  usage = "_FUNC_(struct, name, val) - Adds/replaces field in struct by name.",
+  examples = """
+    Examples:
+      > SELECT _FUNC_(NAMED_STRUCT("a", 1, "b", 2), "c", 3);
+       {"a":1,"b":2,"c":3}
+      > SELECT _FUNC_(NAMED_STRUCT("a", 1, "b", 2), "b", 3);
+       {"a":1,"b":3}
+      > SELECT _FUNC_(CAST(NULL AS struct<a:int,b:int>), "c", 3);
+       {"a":null,"b":null,"c":3}
+      > SELECT _FUNC_(NAMED_STRUCT('a', 1, 'b', 2, 'b', 3), 'b', 100);
+       {"a":1,"b":100,"b":100}
+      > SELECT _FUNC_(a, 'a.c', 3) FROM (VALUES (NAMED_STRUCT('a', 
NAMED_STRUCT('a', 1, 'b', 2))) AS T(a));
+       {"a":{"a":1,"b":2,"c":3}}
+  """)
+// scalastyle:on line.size.limit
+case class WithField(children: Seq[Expression]) extends Unevaluable {
+  private lazy val Seq(structExpr, nestedAttributeExpr, valExpr) = children
+  private lazy val nestedAttribute = nestedAttributeExpr.eval().toString
+  private lazy val attributes = 
UnresolvedAttribute.parseAttributeName(nestedAttribute)
+  lazy val toCreateNamedStruct: Expression = toCreateNamedStruct(structExpr, 
attributes)
+
+  override def dataType: StructType = 
toCreateNamedStruct.dataType.asInstanceOf[StructType]
+
+  override def foldable: Boolean = toCreateNamedStruct.foldable
+
+  override def nullable: Boolean = toCreateNamedStruct.nullable
+
+  override def checkInputDataTypes(): TypeCheckResult = {
+    val structTypeName = StructType(Nil).typeName
+    if (children.size != 3) {
+      TypeCheckResult.TypeCheckFailure(s"$prettyName expects 3 arguments.")
+    } else if (!structExpr.dataType.isInstanceOf[StructType]) {
+      TypeCheckResult.TypeCheckFailure(s"Only $structTypeName is allowed to 
appear at " +
+        s"first position, got: ${structExpr.dataType.typeName}.")
+    } else if (!(nestedAttributeExpr.foldable && nestedAttributeExpr.dataType 
== StringType)) {
+      TypeCheckResult.TypeCheckFailure(s"Only foldable 
${StringType.catalogString} expressions " +
+        s"are allowed to appear at second position.")
+    } else if (nestedAttribute == null) {
+      TypeCheckResult.TypeCheckFailure("Field name should not be null.")
+    } else {
+      val structType = structExpr.dataType.asInstanceOf[StructType]
+      val intermediateAttributes :+ _ = attributes
+      intermediateAttributes

Review comment:
       can we reuse `StructType.findNestedField`?

##########
File path: 
sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala
##########
@@ -923,4 +923,452 @@ class ColumnExpressionSuite extends QueryTest with 
SharedSparkSession {
     val inSet = InSet(Literal("a"), Set("a", "b").map(UTF8String.fromString))
     assert(inSet.sql === "('a' IN ('a', 'b'))")
   }
+
+  {

Review comment:
       do we need to wrap the code with `{...}`?




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

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