Skip to content

[SPARK-24613][SQL] Cache with UDF could not be matched with subsequent dependent caches - #21602

Closed
maryannxue wants to merge 2 commits into
apache:masterfrom
maryannxue:cache-mismatch
Closed

[SPARK-24613][SQL] Cache with UDF could not be matched with subsequent dependent caches#21602
maryannxue wants to merge 2 commits into
apache:masterfrom
maryannxue:cache-mismatch

Conversation

@maryannxue

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Wrap the logical plan with a AnalysisBarrier for execution plan compilation in CacheManager, in order to avoid the plan being analyzed again.

How was this patch tested?

Add one test in DatasetCacheSuite

@SparkQA

Copy link
Copy Markdown

Test build #92149 has finished for PR 21602 at commit 4a5c388.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmilegatorsmile left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

}

test("SPARK-24613 Cache with UDF could not be matched with subsequent dependent caches") {
val expensiveUDF = udf({x: Int => Thread.sleep(10000); x})

@cloud-fancloud-fanJun 21, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we use accumulator and make sure this UDF only run 10 times? sleeping 10 seconds is not good in a unit test

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Accumulators probably wouldn't work. I'll do verify plan though.

@SparkQA

Copy link
Copy Markdown

Test build #92158 has finished for PR 21602 at commit 377f213.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile

Copy link
Copy Markdown
Member

retest this please

@SparkQA

Copy link
Copy Markdown

Test build #92174 has finished for PR 21602 at commit 377f213.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile

gatorsmile commented Jun 21, 2018

Copy link
Copy Markdown
Member

Thanks! Merged to master.

@gatorsmile

Copy link
Copy Markdown
Member

This is also a regression. Backported to 2.3 branch too.

asfgit pushed a commit that referenced this pull request Jun 27, 2018
…t dependent caches
Wrap the logical plan with a `AnalysisBarrier` for execution plan compilation in CacheManager, in order to avoid the plan being analyzed again.
Add one test in `DatasetCacheSuite`
Author: Maryann Xue <maryannxue@apache.org>
Closes#21602 from maryannxue/cache-mismatch.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@maryannxue@SparkQA@gatorsmile@cloud-fan