Re: [PR] [SPARK-57225][SQL] Reject batch writes for DSv2 sources without write capability [spark]

["cloud-fan (via GitHub)" <gi...@apache.org>]

cloud-fan commented on code in PR #56319:
URL: https://github.com/apache/spark/pull/56319#discussion_r3704591875


##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2Suite.scala:
##########
@@ -1687,6 +1687,50 @@ class DataSourceV2Suite extends SharedSparkSession with AdaptiveSparkPlanHelper
     }
   }
 
+  test("SPARK-57225: DSv2 source without batch write capability throws clear error") {
+    val cls = classOf[ReadOnlyV2DataSource].getName
+    val df = spark.range(1).toDF("i")
+    checkError(
+      exception = intercept[AnalysisException] {
+        df.write.format(cls).mode("append").save()
+      },
+      condition = "UNSUPPORTED_FEATURE.TABLE_OPERATION",
+      parameters = Map(
+        "tableName" -> "`read_only_v2_test`",
+        "operation" -> "batch write"
+      )
+    )
+
+    checkError(
+      exception = intercept[AnalysisException] {
+        df.write.format(cls).save()
+      },
+      condition = "UNSUPPORTED_FEATURE.TABLE_OPERATION",
+      parameters = Map(
+        "tableName" -> "`read_only_v2_test`",
+        "operation" -> "batch write"
+      )
+    )
+  }
+
+  test("SPARK-57225: DSv2 source with V1_BATCH_WRITE still falls back to V1 path") {
+    val cls = classOf[V1BatchWriteV2DataSource].getName
+    val df = spark.range(1).toDF("i")
+    // V1_BATCH_WRITE sources should NOT throw unsupported batch write error.
+    // They should fall through to the V1 write path (which may fail for other reasons
+    // like missing CreatableRelationProvider, but NOT with UNSUPPORTED_FEATURE).
+    val ex = intercept[Exception] {
+      df.write.format(cls).mode("append").save()
+    }
+    // Must NOT be UNSUPPORTED_FEATURE.TABLE_OPERATION - that would mean V1 fallback is blocked
+    ex match {
+      case ae: AnalysisException =>
+        assert(ae.getCondition != "UNSUPPORTED_FEATURE.TABLE_OPERATION",
+          "V1_BATCH_WRITE source should not get batch write unsupported error")
+      case _ => // Any other exception is fine - it means V1 path was attempted

Review Comment:
   Please assert the specific `DataSource.planForWriting` failure from this fixture, or another observable V1-path effect. Accepting every other exception lets an unrelated failure pass without proving that `V1_BATCH_WRITE` reached `saveToV1SourceCommand`.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org