Skip to content

[SPARK-22753][SQL] Get rid of dataSource.writeAndRead #19941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xuanyuanking wants to merge 3 commits into from
Closed

[SPARK-22753][SQL] Get rid of dataSource.writeAndRead #19941

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1481163
Get rid of dataSource.writeAndRead
xuanyuanking Dec 11, 2017
41a2c15
Address comments
xuanyuanking Dec 14, 2017
5221c7c
Revert the changes on DataFrameWriter API
xuanyuanking Dec 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ...rc/main/scala/org/apache/spark/sql/execution/command/InsertIntoDataSourceDirCommand.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ case class InsertIntoDataSourceDirCommand(

val saveMode = if (overwrite) SaveMode.Overwrite else SaveMode.ErrorIfExists
try {
sparkSession.sessionState.executePlan(dataSource.planForWriting(saveMode, query))
dataSource.writeAndRead(saveMode, query)
Copy link
Member

@gatorsmile gatorsmile Dec 12, 2017

Choose a reason for hiding this comment

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

How about changing it to

      sparkSession.sessionState.executePlan(dataSource.planForWriting(saveMode, query)).toRdd

Copy link
Member Author

@xuanyuanking xuanyuanking Dec 13, 2017

Choose a reason for hiding this comment

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

I implemented like this at first, but after checking this patch (https://github.com/apache/spark/pull/18064/files), I changed to current implementation, is the wrapping of execution id unnecessary here?

Copy link
Member

@gatorsmile gatorsmile Dec 13, 2017

Choose a reason for hiding this comment

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

I think it is not needed. You can try it.

Copy link
Member Author

@xuanyuanking xuanyuanking Dec 14, 2017

Choose a reason for hiding this comment

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

You're right, test the query "INSERT OVERWRITE DIRECTORY '/home/liyuanjian/tmp' USING json SELECT 1 AS a, 'c' as b;".
image

Copy link
Member

@gatorsmile gatorsmile Dec 14, 2017

Choose a reason for hiding this comment

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

Could we revert all the changes except this?

Copy link
Member Author

@xuanyuanking xuanyuanking Dec 15, 2017

Choose a reason for hiding this comment

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

Revert done.
Sorry, maybe I misunderstand your words of 'get rid of dataSource.writeAndRead'. Like you and Wenchen's discussion in #16481, shouldn't we make writeAndRead just return a BaseRelation without write to the destination? Thank you for your patient reply.

sparkSession.sessionState.executePlan(dataSource.planForWriting(saveMode, query)).toRdd
} catch {
case ex: AnalysisException =>
logError(s"Failed to write to directory " + storage.locationUri.toString, ex)
Expand Down