fqaiser94 commented on a change in pull request #29587:
URL: https://github.com/apache/spark/pull/29587#discussion_r488117655
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala
##########
@@ -546,7 +547,8 @@ case class StringToMap(text: Expression, pairDelim:
Expression, keyValueDelim: E
case class WithFields(
structExpr: Expression,
names: Seq[String],
- valExprs: Seq[Expression]) extends Unevaluable {
+ valExprs: Seq[Expression],
+ sortOutputColumns: Boolean = false) extends Unevaluable {
Review comment:
From an API standpoint, it seems unnatural to me to have `WithFields`
also do sorting. The sorting behaviour feels like a distinctly separate
operation.
Is `sortOutputColumns = true` expected to be common outside of
`ResolveUnion`? If not, perhaps we can keep this logic somewhere else outside
of `WithFields`?
1. Another private helper method in `ResolveUnion`?
2. Another Expression (`OrderStructFieldsByName`)?
Doing so might even help with the aforementioned failing test since you
could re-use this method/Expression on both sides of the union without
necessarily invoking `WithFields`.
----------------------------------------------------------------
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]
