Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Mar 5, 2025

What changes were proposed in this pull request?

This pr change ExplainUtils.generateFieldString to directly call QueryPlan.generateFieldString because these are identical methods.

// TODO(nemanja.petrovic@databricks.com) Delete method as it is duplicated in QueryPlan.scala.
def generateFieldString(fieldName: String, values: Any): String = values match {
case iter: Iterable[_] if (iter.size == 0) => s"${fieldName}: []"
case iter: Iterable[_] => s"${fieldName} [${iter.size}]: ${iter.mkString("[", ", ", "]")}"
case str: String if (str == null || str.isEmpty) => s"${fieldName}: None"
case str: String => s"${fieldName}: ${str}"
case _ => throw new IllegalArgumentException(s"Unsupported type for argument values: $values")
}

def generateFieldString(fieldName: String, values: Any): String = values match {
case iter: Iterable[_] if (iter.size == 0) => s"${fieldName}: []"
case iter: Iterable[_] => s"${fieldName} [${iter.size}]: ${iter.mkString("[", ", ", "]")}"
case str: String if (str == null || str.isEmpty) => s"${fieldName}: None"
case str: String => s"${fieldName}: ${str}"
case _ => throw new IllegalArgumentException(s"Unsupported type for argument values: $values")
}

Meanwhile, this is a TODO left by nemanjapetr-db in SPARK-50739.

Why are the changes needed?

Remove duplicated code.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass Github Actions

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions bot added the SQL label Mar 5, 2025
@LuciferYang LuciferYang marked this pull request as draft March 5, 2025 12:46
@sumitsingh-in
Copy link

LGTM

@LuciferYang LuciferYang marked this pull request as ready for review March 5, 2025 15:10
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Thank you, @LuciferYang .
Merged to master/4.0.

dongjoon-hyun pushed a commit that referenced this pull request Mar 5, 2025
…tly call `QueryPlan.generateFieldString`

### What changes were proposed in this pull request?
This pr change `ExplainUtils.generateFieldString` to directly call `QueryPlan.generateFieldString` because these are identical methods.

https://github.com/apache/spark/blob/4b4bdcfe1fe9bfd38030b855139a12fc55034083/sql/core/src/main/scala/org/apache/spark/sql/execution/ExplainUtils.scala#L300-L307

https://github.com/apache/spark/blob/4b4bdcfe1fe9bfd38030b855139a12fc55034083/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala#L789-L795

Meanwhile, this is a TODO left by nemanjapetr-db  in SPARK-50739.

### Why are the changes needed?
Remove duplicated code.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass Github Actions

### Was this patch authored or co-authored using generative AI tooling?
No

Closes #50169 from LuciferYang/SPARK-51401.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit cdba396)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@LuciferYang LuciferYang deleted the SPARK-51401 branch May 2, 2025 05:24
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 14, 2025
…tly call `QueryPlan.generateFieldString`

### What changes were proposed in this pull request?
This pr change `ExplainUtils.generateFieldString` to directly call `QueryPlan.generateFieldString` because these are identical methods.

https://github.com/apache/spark/blob/eac7ce4dc9e80c4dbaef248f82fc291d856fe046/sql/core/src/main/scala/org/apache/spark/sql/execution/ExplainUtils.scala#L300-L307

https://github.com/apache/spark/blob/eac7ce4dc9e80c4dbaef248f82fc291d856fe046/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala#L789-L795

Meanwhile, this is a TODO left by nemanjapetr-db  in SPARK-50739.

### Why are the changes needed?
Remove duplicated code.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass Github Actions

### Was this patch authored or co-authored using generative AI tooling?
No

Closes apache#50169 from LuciferYang/SPARK-51401.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit daf22c7)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants