c21 commented on a change in pull request #28649:
URL: https://github.com/apache/spark/pull/28649#discussion_r462057050



##########
File path: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala
##########
@@ -166,6 +175,17 @@ case class InsertIntoHiveTable(
       if (isDynamic.init.zip(isDynamic.tail).contains((true, false))) {
         throw new AnalysisException(ErrorMsg.PARTITION_DYN_STA_ORDER.getMsg)
       }
+    } else if (numStaticPartitions > 0) {
+      // scalastyle:off
+      // ifNotExists is only valid with static partition, refer to
+      // 
https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DML#LanguageManualDML-InsertingdataintoHiveTablesfromqueries
+      // scalastyle:on
+      if (!oldPart.isEmpty && ifPartitionNotExists) {
+        val partitionStr = 
partitionSpec.map(_.productIterator.mkString("=")).mkString("/")
+        logWarning(s"Partition '$partitionStr' already exist, skip running job 
to overwrite " +

Review comment:
       +1 for adding a warning message here, I feel previous behavior with 
silently exiting query might be quite confusing for users and developers.
   
   nit: as we could have multiple `INSERT OVERWRITE` in one query, do we also 
want to include table name (`table.identifier.table`) in warning message?

##########
File path: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala
##########
@@ -148,6 +148,15 @@ case class InsertIntoHiveTable(
            |Table partitions: 
${table.partitionColumnNames.mkString(",")}""".stripMargin)
     }
 
+    lazy val oldPart = if ( numDynamicPartitions == 0) {

Review comment:
       +1

##########
File path: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala
##########
@@ -166,6 +175,17 @@ case class InsertIntoHiveTable(
       if (isDynamic.init.zip(isDynamic.tail).contains((true, false))) {
         throw new AnalysisException(ErrorMsg.PARTITION_DYN_STA_ORDER.getMsg)
       }
+    } else if (numStaticPartitions > 0) {
+      // scalastyle:off
+      // ifNotExists is only valid with static partition, refer to
+      // 
https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DML#LanguageManualDML-InsertingdataintoHiveTablesfromqueries
+      // scalastyle:on
+      if (!oldPart.isEmpty && ifPartitionNotExists) {

Review comment:
       nit: `if (oldPart.isDefined && ifPartitionNotExists)`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to