[SPARK-15706] [SQL] Fix Wrong Answer when using IF NOT EXISTS in INSERT OVERWRITE for DYNAMIC PARTITION

[gatorsmile]

What changes were proposed in this pull request?

IF NOT EXISTS in INSERT OVERWRITE should not support dynamic partitions. If we specify IF NOT EXISTS, the inserted statement is not shown in the table.

This PR is to issue an exception in this case, just like what Hive does. Also issue an exception if users specify IF NOT EXISTS if users do not specify any PARTITION specification.

How was this patch tested?

Added test cases into PlanParserSuite and InsertIntoHiveTableSuite

[SparkQA]

Test build #59753 has finished for PR 13447 at commit 61f0632.

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

[SparkQA]

Test build #59755 has finished for PR 13447 at commit 70d9cad.

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

[gatorsmile]

cc @liancheng @hvanhovell @yhuai

[hvanhovell]

We could enforce this in grammar.

[gatorsmile]

Sure, will do it. Thanks!

[hvanhovell]

@gatorsmile the overall approach seems good to me. We are currently fixing this for the SQL codepath. I was wondering if are other codepaths that can cause this unwanted behavior? If there are then we should move the check to the InsertInto class.

[gatorsmile]

I think your concern is valid. Will add an assert in InsertIntoTable.

So far, dynamic partitioning is used by the insertInto API. However, there is no way to specify IF NOT EXISTS in the other API insertInto.

data.write.mode(SaveMode.Overwrite).partitionBy("part").insertInto("partitioned")

The default value is always false, as shown below:

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

Line 454 in a71d136

ifNotExists = false)).toRdd

However, there still exist other bugs in insertInto. Will submit it later.

[SparkQA]

Test build #59865 has finished for PR 13447 at commit be187d8.

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

[SparkQA]

Test build #59873 has finished for PR 13447 at commit bbaad66.

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

[gatorsmile]

retest this please

[SparkQA]

Test build #60098 has finished for PR 13447 at commit bbaad66.

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

[gatorsmile]

@hvanhovell Does the latest code changes resolve your comment? Thanks!

[yhuai]

Seems the behavior of the following query is weird (if it works). (the case without IF NOT EXISTS)

INSERT OVERWRITE TABLE table_with_partition
partition (p1='a',p2)
SELECT 'blarr3'

For this case, the number of column (1) of the query is less than the required number of columns (2). There should be an exception, right?

[gatorsmile]

@yhuai You are right. This is not a good test case for verifying this. I will add a case like

      sql(
        """
          |INSERT OVERWRITE TABLE table_with_partition
          |partition (p1='a',p2) IF NOT EXISTS
          |SELECT 'blarr3', 'newPartition'
        """.stripMargin)

The above statement returns an empty result set, which is still a wrong answer.

Regarding your follow-up question, I also realized this is another bug we should capture, as shown above: #13447 (comment). My original plan is to submit a PR for addressing that issue after #12313 is merged. It sounds like we do not plan to do this in 2.0: #12313, I can do it now. Thanks!

[SparkQA]

Test build #60344 has finished for PR 13447 at commit 84ed71a.

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

[gatorsmile]

retest this please

[SparkQA]

Test build #60619 has finished for PR 13447 at commit 84ed71a.

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

[gatorsmile]

cc @hvanhovell @yhuai Could you please review this PR again? Thanks!

[yhuai]

Thanks. LGTM. Merging to master and branch 2.0.