Skip to content

Conversation

@zzzzming95
Copy link
Contributor

What changes were proposed in this pull request?

Trigger committer.setupJob before plan execute in FileFormatWriter#write

Why are the changes needed?

In this issue, the case where outputOrdering might not work if AQE is enabled has been resolved.

#38358

However, since it materializes the AQE plan in advance (triggers getFinalPhysicalPlan) , it may cause the committer.setupJob(job) to not execute When AdaptiveSparkPlanExec#getFinalPhysicalPlan() is executed with an error.

Does this PR introduce any user-facing change?

no

How was this patch tested?

add UT

@github-actions github-actions bot added the SQL label May 12, 2023
}
}

test("SPARK-43327: location exists when insertoverwrite fails") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This issue did not occur in the latest master branch, only in Spark3.2 and Spark3.3

@HyukjinKwon HyukjinKwon changed the title [SPARK-43327] Trigger committer.setupJob before plan execute in FileFormatWriter#write [SPARK-43327][CORE] Trigger committer.setupJob before plan execute in FileFormatWriter#write May 15, 2023
@HyukjinKwon HyukjinKwon changed the title [SPARK-43327][CORE] Trigger committer.setupJob before plan execute in FileFormatWriter#write [SPARK-43327][CORE][3.3] Trigger committer.setupJob before plan execute in FileFormatWriter#write May 15, 2023
@HyukjinKwon
Copy link
Member

@zzzzming95 do you know in which PR fixed the issue in master branch?

@zzzzming95
Copy link
Contributor Author

zzzzming95 commented May 21, 2023

do you know in which PR fixed the issue in master branch?
@HyukjinKwon

The direct reason is because SPARK-37287,

It causes' DataWriteingCommandExec 'to be encapsulated in' AdaptiveSparkPlanExec 'and triggers execution in advance.

mabay we can just backport SPARK-37287 to spark-3.3 ?

@HyukjinKwon @cloud-fan cc

@cloud-fan
Copy link
Contributor

SPARK-37287 is too big to backport

sql("create table t1(c2 long) using parquet")
sql("INSERT OVERWRITE TABLE t1 select 6000044164")

// spark.sql("CREATE TABLE IF NOT EXISTS t(amt1 int) using ORC")
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@zzzzming95 zzzzming95 requested a review from cloud-fan August 12, 2023 02:42
@zzzzming95
Copy link
Contributor Author

@cloud-fan

do you know how to make it run green ? I see all test pass. Or it can merge directly? please take a look , thanks~

@cloud-fan
Copy link
Contributor

yea all tests passed, let me just merge it. Thanks!

cloud-fan pushed a commit that referenced this pull request Aug 22, 2023
…cute in `FileFormatWriter#write`

### What changes were proposed in this pull request?

Trigger `committer.setupJob` before plan execute in `FileFormatWriter#write`

### Why are the changes needed?

In this issue, the case where `outputOrdering` might not work if AQE is enabled has been resolved.

#38358

However, since it materializes the AQE plan in advance (triggers getFinalPhysicalPlan) , it may cause the committer.setupJob(job) to not execute When `AdaptiveSparkPlanExec#getFinalPhysicalPlan()` is executed with an error.

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

no

### How was this patch tested?

add UT

Closes #41154 from zzzzming95/spark3-SPARK-43327.

Lead-authored-by: zzzzming95 <[email protected]>
Co-authored-by: zhiming she <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
@cloud-fan cloud-fan closed this Aug 22, 2023
@zzzzming95
Copy link
Contributor Author

@cloud-fan
thanks for you review ~

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