[SPARK-16006][SQL] Attemping to write empty DataFrame with no fields throws non-intuitive exception

[dongjoon-hyun]

What changes were proposed in this pull request?

This PR allows emptyDataFrame.write since the user didn't specify any partition columns.

Before

scala> spark.emptyDataFrame.write.parquet("/tmp/t1")
org.apache.spark.sql.AnalysisException: Cannot use all columns for partition columns;
scala> spark.emptyDataFrame.write.csv("/tmp/t1")
org.apache.spark.sql.AnalysisException: Cannot use all columns for partition columns;

After this PR, there occurs no exceptions and the created directory has only one file, _SUCCESS, as expected.

How was this patch tested?

Pass the Jenkins tests including updated test cases.

[dongjoon-hyun]

Since validatePartitionColumn is used for only writing-related classes, I added this description for clarification.
We can update the function description if the usage pattern is changed in the future.

[dongjoon-hyun]

Hi, @tdas .
This is the PR to care the reported corner case.

[SparkQA]

Test build #60686 has finished for PR 13730 at commit da1017e.

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

[tdas]

Its weird that a check like this is in the PartitioningUtils. This check seems nothing to do with partitioning, as its basically certain file formats that do not support writing a DF with no columns. Is there somewhere earlier where you can check this?

[dongjoon-hyun]

Thank you for review, @tdas.
Yes. Indeed. This is beyond of the scope of PartitioningUtils.
Actually, this logic is used 3 classes, PreWriteCheck., DataSource, FileStreamSinkWriter
I'll try to move this.

[dongjoon-hyun]

Hi, @tdas .
At first look, I thought this corner case needs to throw exceptions.
But, after considering more carefully, I want to allow emptyDataFrame.write without exceptions.
That is more natural way to handle this corner case because the user didn't give any partition columns.
How do you think about this?

(Anyway, I will update this PR for further discussion)

[SparkQA]

Test build #60704 has finished for PR 13730 at commit 5f1db7f.

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

[dongjoon-hyun]

Hi, @tdas .
Could you review this PR again when you have some time?

[dongjoon-hyun]

Hi, @rxin .
Could you review this PR ?

[dongjoon-hyun]

Oh, sorry. The master was changed.

[dongjoon-hyun]

I will recheck this PR again.

[dongjoon-hyun]

Yep. The case still exists for parquet/csv and I updated the cases.
The previous text case changes like the following and looks legitimate.

scala> spark.emptyDataFrame.write.text("/tmp/t1")
org.apache.spark.sql.AnalysisException: Text data source supports only a single column, and you have 0 columns.;

[dongjoon-hyun]

Hi, @tdas .
Could you review this PR when you have some time?
Current master still has this issue.

[SparkQA]

Test build #61014 has finished for PR 13730 at commit fd0b1cf.

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

[dongjoon-hyun]

Hi, @tdas .
Could you give me some advice for the direction about how to change this PR?

[dongjoon-hyun]

Rebased.

[SparkQA]

Test build #61183 has finished for PR 13730 at commit 7bb64d7.

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

[SparkQA]

Test build #61254 has finished for PR 13730 at commit ba9a529.

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

[dongjoon-hyun]

Ping @tdas

[SparkQA]

Test build #61391 has started for PR 13730 at commit 7d38003.

[dongjoon-hyun]

Retest this please.

[SparkQA]

Test build #61397 has finished for PR 13730 at commit 7d38003.

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

[dongjoon-hyun]

Hi, @tdas .
Could you give me some advice on this issue?

[rxin]

Thanks - merging in master/2.0.

[dongjoon-hyun]

Thank you for merging, @rxin .