[SPARK-16236] [SQL] Add Path Option back to Load API in DataFrameReader

[gatorsmile]

What changes were proposed in this pull request?

@koertkuipers identified the PR #13727 changed the behavior of load API. After the change, the load API does not add the value of path into the options. Thank you!

This PR is to add the option path back to load() API in DataFrameReader, if and only if users specify one and only one path in the load API. For example, users can see the path option after the following API call,

spark.read
  .format("parquet")
  .load("/test")

How was this patch tested?

Added test cases.

[gatorsmile]

cc @rxin @tdas @koertkuipers

[SparkQA]

Test build #61333 has finished for PR 13933 at commit 15eb528.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #61342 has finished for PR 13933 at commit bf1c9b5.

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

[gatorsmile]

cc @rxin The code is ready for review. Thanks!

[rxin]

LGTM -- cc @tdas to take a look since he wrote the original patch.

[liancheng]

LGTM.

[rxin]

Merging in master/2.0.

[zsxwing]

I noticed that the Python API is inconsistent here:

spark/python/pyspark/sql/readwriter.py

Line 147 in 1aad8c6

if type(path) != list:

It always calls def load(paths: String*).

[gatorsmile]

@zsxwing Let me try to fix it. Thanks!

[gatorsmile]

The problem also exists in the other APIs:
like parquet, json and others.

spark/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala

Lines 407 to 410 in 25520e9

	def parquet(path: String): DataFrame = {
	// This method ensures that calls that explicit need single argument works, see SPARK-16009
	parquet(Seq(path): _*)
	}

Let me fix them in the same PR. Thanks!

[zsxwing]

@gatorsmile parquet, json or other file formats support both path and paths options. So that's not a problem.

[gatorsmile]

@zsxwing If we just provide one path in the function input, it will not put path into the options. The API parquet(path: String)still calls load(paths : _*), instead of load(path). Thus, we will introduce inconsistent behavior, compared with load(path: String).

Could you review the new PR I just submitted? Let me know if anything is not appropriate. #13965. Thanks!

[koertkuipers]

For parquet, json etc. path not being put in options is not an issue since
they don't retrieve it from the options
On Jun 29, 2016 2:31 AM, "Xiao Li" [email protected] wrote:

@zsxwing https://github.com/zsxwing If we just provide one path in the
function input, it will not put path into the options. The API parquet(path:
String)still calls load(paths : _*), instead of load(path). Thus, we will
introduce inconsistent behavior, compared with load(path: String).

Could you review the new PR I just submitted? Let me know if anything is
not appropriate. #13965 #13965.
Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13933 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAyIJPT3agmWQyqBUqrzFW5F2eE095Wtks5qQhFPgaJpZM4I_nrj
.