Skip to content

Commit

Permalink
[SPARK-45242][SQL][FOLLOWUP] Canonicalize DataFrame ID in CollectMetrics
Browse files Browse the repository at this point in the history
### What changes were proposed in this pull request?

In #43010, a new DataFrameId field is added to `CollectMetrics`. We should also canonicalize the new DataFrame id field to avoid downstream plan comparison failures.

### Why are the changes needed?

avoid downstream plan comparison failures.

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

No

### How was this patch tested?

UT

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

NO

Closes #43594 from amaliujia/do_not_canonicalize_dataframe_id.

Authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
  • Loading branch information
amaliujia authored and gengliangwang committed Oct 31, 2023
1 parent 49f9e74 commit af8907a
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1983,6 +1983,10 @@ case class CollectMetrics(

override protected def withNewChildInternal(newChild: LogicalPlan): CollectMetrics =
copy(child = newChild)

override def doCanonicalize(): LogicalPlan = {
super.doCanonicalize().asInstanceOf[CollectMetrics].copy(dataframeId = 0L)
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,11 @@ class AnalysisSuite extends AnalysisTest with Matchers {
)
}

test("Canonicalize CollectMetrics") {
assert(CollectMetrics("", Seq(Rand(10).as("rnd")), testRelation, 1).canonicalized ==
CollectMetrics("", Seq(Rand(10).as("rnd")), testRelation, 2).canonicalized)
}

test("Analysis exceed max iterations") {
// RuleExecutor only throw exception or log warning when the rule is supposed to run
// more than once.
Expand Down

0 comments on commit af8907a

Please sign in to comment.