You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ueshin (via GitHub)" <gi...@apache.org> on 2025/01/06 21:49:48 UTC
[PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
ueshin opened a new pull request, #49386:
URL: https://github.com/apache/spark/pull/49386
### What changes were proposed in this pull request?
Supports `withColumns` / `withColumnsRenamed` in subqueries.
### Why are the changes needed?
When the query is used as a subquery by adding `col.outer()`, `withColumns` or `withColumnsRenamed` doesn't work because they need analyzed plans.
### Does this PR introduce _any_ user-facing change?
Yes, those APIs are available in subqueries.
### How was this patch tested?
Added the related tests.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50601][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #49386:
URL: https://github.com/apache/spark/pull/49386#issuecomment-2588705330
The remaining test failures are not related to this PR.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1904898782
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -1071,3 +1072,29 @@ trait UnresolvedPlanId extends LeafExpression with Unevaluable {
// Subclasses can override this function to provide more TreePatterns.
def nodePatternsInternal(): Seq[TreePattern] = Seq()
}
+
+case class UnresolvedWithColumns(
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ metadata: Option[Seq[Metadata]],
+ child: LogicalPlan)
+ extends UnresolvedUnaryNode {
+
+ final override val nodePatterns: Seq[TreePattern] = Seq(UNRESOLVED_WITH_COLUMNS)
+
+ override protected def withNewChildInternal(
+ newChild: LogicalPlan): UnresolvedWithColumns = copy(child = newChild)
+}
+
+case class UnresolvedWithColumnsRenamed(
Review Comment:
oh, `df.withColumn` can append or replace columns, we should probably extend `UnresolvedStarExceptOrReplace` for this case.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1904897052
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -1071,3 +1072,29 @@ trait UnresolvedPlanId extends LeafExpression with Unevaluable {
// Subclasses can override this function to provide more TreePatterns.
def nodePatternsInternal(): Seq[TreePattern] = Seq()
}
+
+case class UnresolvedWithColumns(
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ metadata: Option[Seq[Metadata]],
+ child: LogicalPlan)
+ extends UnresolvedUnaryNode {
+
+ final override val nodePatterns: Seq[TreePattern] = Seq(UNRESOLVED_WITH_COLUMNS)
+
+ override protected def withNewChildInternal(
+ newChild: LogicalPlan): UnresolvedWithColumns = copy(child = newChild)
+}
+
+case class UnresolvedWithColumnsRenamed(
Review Comment:
We can reuse `Project` with `UnresolvedStarExceptOrReplace`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1906451581
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -1071,3 +1072,29 @@ trait UnresolvedPlanId extends LeafExpression with Unevaluable {
// Subclasses can override this function to provide more TreePatterns.
def nodePatternsInternal(): Seq[TreePattern] = Seq()
}
+
+case class UnresolvedWithColumns(
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ metadata: Option[Seq[Metadata]],
+ child: LogicalPlan)
+ extends UnresolvedUnaryNode {
+
+ final override val nodePatterns: Seq[TreePattern] = Seq(UNRESOLVED_WITH_COLUMNS)
+
+ override protected def withNewChildInternal(
+ newChild: LogicalPlan): UnresolvedWithColumns = copy(child = newChild)
+}
+
+case class UnresolvedWithColumnsRenamed(
Review Comment:
I see. One question is shall we add new logical plans or new star-like expressions? star-like expressions are more flexible as they work for function input as well (e.g. `struct(*)`). And we might be able to unify them with the SQL pipe one with extra flags.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #49386:
URL: https://github.com/apache/spark/pull/49386#issuecomment-2574176609
cc @cloud-fan too
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1909794832
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -712,6 +717,109 @@ case class UnresolvedStarExceptOrReplace(
}
}
+/**
+ * Represents some of the input attributes to a given relational operator, for example in
+ * `df.withColumn`.
+ *
+ * @param target an optional name that should be the target of the expansion. If omitted all
+ * targets' columns are produced. This can only be a table name. This
+ * is a list of identifiers that is the path of the expansion.
+ *
+ * @param colNames a list of column names that should be replaced or produced.
+ *
+ * @param exprs a list of expressions that should be used to replace the expressions removed by.
Review Comment:
```suggestion
* @param exprs the corresponding expressions for `colNames`.
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1905993989
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -1071,3 +1072,29 @@ trait UnresolvedPlanId extends LeafExpression with Unevaluable {
// Subclasses can override this function to provide more TreePatterns.
def nodePatternsInternal(): Seq[TreePattern] = Seq()
}
+
+case class UnresolvedWithColumns(
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ metadata: Option[Seq[Metadata]],
+ child: LogicalPlan)
+ extends UnresolvedUnaryNode {
+
+ final override val nodePatterns: Seq[TreePattern] = Seq(UNRESOLVED_WITH_COLUMNS)
+
+ override protected def withNewChildInternal(
+ newChild: LogicalPlan): UnresolvedWithColumns = copy(child = newChild)
+}
+
+case class UnresolvedWithColumnsRenamed(
Review Comment:
There seem to be some semantic differences even with `withColumnsRenamed`:
- If the specified column names are missing in the `df`, `df.withColumnsRenamed` ignores whereas `UnresolvedStarExceptOrReplace` throws an exception.
- `df.withColumnsRenamed` respects the argument order, e.g.,
```
test("SPARK-46260: withColumnsRenamed should respect the Map ordering") {
val df = spark.range(10).toDF()
assert(df.withColumnsRenamed(ListMap("id" -> "a", "a" -> "b")).columns === Array("b"))
assert(df.withColumnsRenamed(ListMap("a" -> "b", "id" -> "a")).columns === Array("a"))
}
```
whereas `UnresolvedStarExceptOrReplace` throws an exception.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1911767455
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -712,6 +717,109 @@ case class UnresolvedStarExceptOrReplace(
}
}
+/**
+ * Represents some of the input attributes to a given relational operator, for example in
+ * `df.withColumn`.
+ *
+ * @param target an optional name that should be the target of the expansion. If omitted all
+ * targets' columns are produced. This can only be a table name. This
+ * is a list of identifiers that is the path of the expansion.
+ *
+ * @param colNames a list of column names that should be replaced or produced.
+ *
+ * @param exprs a list of expressions that should be used to replace the expressions removed by.
+ *
+ * @param explicitMetadata an optional list of explicit metadata to associate with the columns.
+ */
+case class UnresolvedStarWithColumns(
+ target: Option[Seq[String]],
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ explicitMetadata: Option[Seq[Metadata]] = None)
+ extends UnresolvedStarBase {
+
+ override def children: Seq[Expression] = exprs
+
+ override protected def withNewChildrenInternal(
+ newChildren: IndexedSeq[Expression]): UnresolvedStarWithColumns =
+ copy(exprs = newChildren)
+
+ override def expand(input: LogicalPlan, resolver: Resolver): Seq[NamedExpression] = {
+ assert(colNames.size == exprs.size,
Review Comment:
I think if we don't remove the above check, assertions are enough as same as what `UnresolvedStarExceptOrReplace` does for `replacements`.
Otherwise, yes, let's add a new error class.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1909797114
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -712,6 +717,109 @@ case class UnresolvedStarExceptOrReplace(
}
}
+/**
+ * Represents some of the input attributes to a given relational operator, for example in
+ * `df.withColumn`.
+ *
+ * @param target an optional name that should be the target of the expansion. If omitted all
+ * targets' columns are produced. This can only be a table name. This
+ * is a list of identifiers that is the path of the expansion.
+ *
+ * @param colNames a list of column names that should be replaced or produced.
+ *
+ * @param exprs a list of expressions that should be used to replace the expressions removed by.
+ *
+ * @param explicitMetadata an optional list of explicit metadata to associate with the columns.
+ */
+case class UnresolvedStarWithColumns(
+ target: Option[Seq[String]],
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ explicitMetadata: Option[Seq[Metadata]] = None)
+ extends UnresolvedStarBase {
+
+ override def children: Seq[Expression] = exprs
+
+ override protected def withNewChildrenInternal(
+ newChildren: IndexedSeq[Expression]): UnresolvedStarWithColumns =
+ copy(exprs = newChildren)
+
+ override def expand(input: LogicalPlan, resolver: Resolver): Seq[NamedExpression] = {
+ assert(colNames.size == exprs.size,
+ s"The size of column names: ${colNames.size} isn't equal to " +
+ s"the size of expressions: ${exprs.size}")
+ explicitMetadata.foreach { m =>
+ assert(colNames.size == m.size,
+ s"The size of column names: ${colNames.size} isn't equal to " +
+ s"the size of metadata elements: ${m.size}")
+ }
+
+ SchemaUtils.checkColumnNameDuplication(colNames, resolver)
+
+ val expandedCols = super.expand(input, resolver)
+
+ val columnSeq = explicitMetadata match {
+ case Some(ms) => colNames.zip(exprs).zip(ms.map(Some(_)))
+ case _ => colNames.zip(exprs).map((_, None))
+ }
+
+ val replacedAndExistingColumns = expandedCols.map { field =>
+ columnSeq.find { case ((colName, _), _) =>
+ resolver(field.name, colName)
+ } match {
+ case Some(((colName, expr), m)) => Alias(expr, colName)(explicitMetadata = m)
+ case _ => field
+ }
+ }
+
+ val newColumns = columnSeq.filter { case ((colName, _), _) =>
+ !expandedCols.exists(f => resolver(f.name, colName))
+ }.map {
+ case ((colName, expr), m) => Alias(expr, colName)(explicitMetadata = m)
+ }
+
+ replacedAndExistingColumns ++ newColumns
+ }
+}
+
+/**
+ * Represents some of the input attributes to a given relational operator, for example in
+ * `df.withColumnRenamed`.
+ *
+ * @param target an optional name that should be the target of the expansion. If omitted all
+ * targets' columns are produced. This can only be a table name. This
+ * is a list of identifiers that is the path of the expansion.
+ *
+ * @param existingNames a list of column names that should be replaced.
Review Comment:
shall we mention that if the column does not exist, we don't replace and just leave it?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -712,6 +717,109 @@ case class UnresolvedStarExceptOrReplace(
}
}
+/**
+ * Represents some of the input attributes to a given relational operator, for example in
+ * `df.withColumn`.
+ *
+ * @param target an optional name that should be the target of the expansion. If omitted all
+ * targets' columns are produced. This can only be a table name. This
+ * is a list of identifiers that is the path of the expansion.
+ *
+ * @param colNames a list of column names that should be replaced or produced.
+ *
+ * @param exprs a list of expressions that should be used to replace the expressions removed by.
+ *
+ * @param explicitMetadata an optional list of explicit metadata to associate with the columns.
+ */
+case class UnresolvedStarWithColumns(
+ target: Option[Seq[String]],
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ explicitMetadata: Option[Seq[Metadata]] = None)
+ extends UnresolvedStarBase {
+
+ override def children: Seq[Expression] = exprs
+
+ override protected def withNewChildrenInternal(
+ newChildren: IndexedSeq[Expression]): UnresolvedStarWithColumns =
+ copy(exprs = newChildren)
+
+ override def expand(input: LogicalPlan, resolver: Resolver): Seq[NamedExpression] = {
+ assert(colNames.size == exprs.size,
+ s"The size of column names: ${colNames.size} isn't equal to " +
+ s"the size of expressions: ${exprs.size}")
+ explicitMetadata.foreach { m =>
+ assert(colNames.size == m.size,
+ s"The size of column names: ${colNames.size} isn't equal to " +
+ s"the size of metadata elements: ${m.size}")
+ }
+
+ SchemaUtils.checkColumnNameDuplication(colNames, resolver)
+
+ val expandedCols = super.expand(input, resolver)
+
+ val columnSeq = explicitMetadata match {
+ case Some(ms) => colNames.zip(exprs).zip(ms.map(Some(_)))
+ case _ => colNames.zip(exprs).map((_, None))
+ }
+
+ val replacedAndExistingColumns = expandedCols.map { field =>
+ columnSeq.find { case ((colName, _), _) =>
+ resolver(field.name, colName)
+ } match {
+ case Some(((colName, expr), m)) => Alias(expr, colName)(explicitMetadata = m)
+ case _ => field
+ }
+ }
+
+ val newColumns = columnSeq.filter { case ((colName, _), _) =>
+ !expandedCols.exists(f => resolver(f.name, colName))
+ }.map {
+ case ((colName, expr), m) => Alias(expr, colName)(explicitMetadata = m)
+ }
+
+ replacedAndExistingColumns ++ newColumns
+ }
+}
+
+/**
+ * Represents some of the input attributes to a given relational operator, for example in
+ * `df.withColumnRenamed`.
+ *
+ * @param target an optional name that should be the target of the expansion. If omitted all
+ * targets' columns are produced. This can only be a table name. This
+ * is a list of identifiers that is the path of the expansion.
+ *
+ * @param existingNames a list of column names that should be replaced.
+ *
+ * @param newNames a list of new column names that should be used to replace the existing columns.
+ */
+case class UnresolvedStarWithColumnsRenames(
+ target: Option[Seq[String]],
Review Comment:
ditto, not used.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1905993989
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -1071,3 +1072,29 @@ trait UnresolvedPlanId extends LeafExpression with Unevaluable {
// Subclasses can override this function to provide more TreePatterns.
def nodePatternsInternal(): Seq[TreePattern] = Seq()
}
+
+case class UnresolvedWithColumns(
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ metadata: Option[Seq[Metadata]],
+ child: LogicalPlan)
+ extends UnresolvedUnaryNode {
+
+ final override val nodePatterns: Seq[TreePattern] = Seq(UNRESOLVED_WITH_COLUMNS)
+
+ override protected def withNewChildInternal(
+ newChild: LogicalPlan): UnresolvedWithColumns = copy(child = newChild)
+}
+
+case class UnresolvedWithColumnsRenamed(
Review Comment:
There seem to be some semantic differences even with `withColumnsRenamed`:
- If the specified column names are missing in the `df`, `df.withColumnsRenamed` ignores whereas `UnresolvedStarExceptOrReplace` throws an exception.
- `df.withColumnsRenamed` respects the argument order, e.g.,
```
test("SPARK-46260: withColumnsRenamed should respect the Map ordering") {
val df = spark.range(10).toDF()
assert(df.withColumnsRenamed(ListMap("id" -> "a", "a" -> "b")).columns === Array("b"))
assert(df.withColumnsRenamed(ListMap("a" -> "b", "id" -> "a")).columns === Array("a"))
}
```
whereas `UnresolvedStarExceptOrReplace` throws an exception.
I guess we should keep the new plans to be safer?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1907992185
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -1071,3 +1072,29 @@ trait UnresolvedPlanId extends LeafExpression with Unevaluable {
// Subclasses can override this function to provide more TreePatterns.
def nodePatternsInternal(): Seq[TreePattern] = Seq()
}
+
+case class UnresolvedWithColumns(
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ metadata: Option[Seq[Metadata]],
+ child: LogicalPlan)
+ extends UnresolvedUnaryNode {
+
+ final override val nodePatterns: Seq[TreePattern] = Seq(UNRESOLVED_WITH_COLUMNS)
+
+ override protected def withNewChildInternal(
+ newChild: LogicalPlan): UnresolvedWithColumns = copy(child = newChild)
+}
+
+case class UnresolvedWithColumnsRenamed(
Review Comment:
Moved to the star-like expressions.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1909799001
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -712,6 +717,109 @@ case class UnresolvedStarExceptOrReplace(
}
}
+/**
+ * Represents some of the input attributes to a given relational operator, for example in
+ * `df.withColumn`.
+ *
+ * @param target an optional name that should be the target of the expansion. If omitted all
+ * targets' columns are produced. This can only be a table name. This
+ * is a list of identifiers that is the path of the expansion.
+ *
+ * @param colNames a list of column names that should be replaced or produced.
+ *
+ * @param exprs a list of expressions that should be used to replace the expressions removed by.
+ *
+ * @param explicitMetadata an optional list of explicit metadata to associate with the columns.
+ */
+case class UnresolvedStarWithColumns(
+ target: Option[Seq[String]],
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ explicitMetadata: Option[Seq[Metadata]] = None)
+ extends UnresolvedStarBase {
+
+ override def children: Seq[Expression] = exprs
+
+ override protected def withNewChildrenInternal(
+ newChildren: IndexedSeq[Expression]): UnresolvedStarWithColumns =
+ copy(exprs = newChildren)
+
+ override def expand(input: LogicalPlan, resolver: Resolver): Seq[NamedExpression] = {
+ assert(colNames.size == exprs.size,
Review Comment:
This is a user-facing name, as these two lists are provided by users. Shall we add a new error class for it?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1909795652
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -712,6 +717,109 @@ case class UnresolvedStarExceptOrReplace(
}
}
+/**
+ * Represents some of the input attributes to a given relational operator, for example in
+ * `df.withColumn`.
+ *
+ * @param target an optional name that should be the target of the expansion. If omitted all
+ * targets' columns are produced. This can only be a table name. This
+ * is a list of identifiers that is the path of the expansion.
+ *
+ * @param colNames a list of column names that should be replaced or produced.
+ *
+ * @param exprs a list of expressions that should be used to replace the expressions removed by.
+ *
+ * @param explicitMetadata an optional list of explicit metadata to associate with the columns.
+ */
+case class UnresolvedStarWithColumns(
+ target: Option[Seq[String]],
Review Comment:
This is not used at all in this expression. We should remove it from the parameter list, and just override it to always return `None`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1911766156
##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -1275,29 +1275,15 @@ class Dataset[T] private[sql](
require(colNames.size == cols.size,
Review Comment:
I think it's still good to have to be fail-fast, especially for connect?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50601][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #49386:
URL: https://github.com/apache/spark/pull/49386#issuecomment-2584956725
`SparkConnectPlanner` has an issue. Waiting for #49449.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1904898889
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -1071,3 +1072,29 @@ trait UnresolvedPlanId extends LeafExpression with Unevaluable {
// Subclasses can override this function to provide more TreePatterns.
def nodePatternsInternal(): Seq[TreePattern] = Seq()
}
+
+case class UnresolvedWithColumns(
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ metadata: Option[Seq[Metadata]],
+ child: LogicalPlan)
+ extends UnresolvedUnaryNode {
+
+ final override val nodePatterns: Seq[TreePattern] = Seq(UNRESOLVED_WITH_COLUMNS)
+
+ override protected def withNewChildInternal(
+ newChild: LogicalPlan): UnresolvedWithColumns = copy(child = newChild)
+}
+
+case class UnresolvedWithColumnsRenamed(
Review Comment:
cc @dtenedor
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1909798456
##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -1275,29 +1275,15 @@ class Dataset[T] private[sql](
require(colNames.size == cols.size,
Review Comment:
Is this check still needed?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50601][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin closed pull request #49386: [SPARK-50601][SQL] Support withColumns / withColumnsRenamed in subqueries
URL: https://github.com/apache/spark/pull/49386
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50601][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #49386:
URL: https://github.com/apache/spark/pull/49386#issuecomment-2588706930
Thanks! merging to master.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50694][SQL] Support withColumns / withColumnsRenamed in subqueries [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49386:
URL: https://github.com/apache/spark/pull/49386#discussion_r1904896615
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala:
##########
@@ -1071,3 +1072,29 @@ trait UnresolvedPlanId extends LeafExpression with Unevaluable {
// Subclasses can override this function to provide more TreePatterns.
def nodePatternsInternal(): Seq[TreePattern] = Seq()
}
+
+case class UnresolvedWithColumns(
+ colNames: Seq[String],
+ exprs: Seq[Expression],
+ metadata: Option[Seq[Metadata]],
+ child: LogicalPlan)
+ extends UnresolvedUnaryNode {
+
+ final override val nodePatterns: Seq[TreePattern] = Seq(UNRESOLVED_WITH_COLUMNS)
+
+ override protected def withNewChildInternal(
+ newChild: LogicalPlan): UnresolvedWithColumns = copy(child = newChild)
+}
+
+case class UnresolvedWithColumnsRenamed(
Review Comment:
I don't think we need new logical plans. See how we implement a similar feature for SQL pipe: https://github.com/apache/spark/commit/68be1da5ba8e54a67eb61822cdf7f8eb47926319
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org