Re: [PR] [SPARK-58389][SQL] Pass table-state options while loading write targets [spark]

["yyanyy (via GitHub)" <gi...@apache.org>]

yyanyy commented on code in PR #57865:
URL: https://github.com/apache/spark/pull/57865#discussion_r3780505803


##########
sql/core/src/test/scala/org/apache/spark/sql/connector/AppendDataTransactionSuite.scala:
##########
@@ -19,14 +19,41 @@ package org.apache.spark.sql.connector
 
 import org.apache.spark.sql.AnalysisException
 import org.apache.spark.sql.Row
-import org.apache.spark.sql.connector.catalog.{Aborted, Committed}
+import org.apache.spark.sql.connector.catalog.{Aborted, Committed, TableWritePrivilege, Txn}
 import org.apache.spark.sql.functions._
 import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.sql.internal.SQLConf.PartitionOverwriteMode
 import org.apache.spark.sql.sources
 
 class AppendDataTransactionSuite extends RowLevelOperationSuiteBase {
 
+  private val targetLoadOption = "targetLoadOption"
+  private val targetLoadValue = "loadValue"
+  private val targetWriteOption = "targetWriteOption"
+  private val targetWriteValue = "writeValue"
+  private val targetOptionsClause =
+    s"WITH (`$targetLoadOption` = '$targetLoadValue', " +
+      s"`$targetWriteOption` = '$targetWriteValue')"
+
+  private def assertTargetLoadAndWriteOptions(
+      txn: Txn,
+      expectedPrivileges: java.util.Set[TableWritePrivilege],
+      minTargetLoads: Int = 1): Unit = {
+    val targetLoads = txn.catalog.loadTableCalls.filter {
+      case (_, options) => options.get(targetLoadOption) == targetLoadValue
+    }
+    assert(targetLoads.size >= minTargetLoads,
+      s"expected at least $minTargetLoads target loads with write options")
+    targetLoads.foreach { case (context, options) =>
+      assert(context.writePrivileges() === expectedPrivileges)
+      assert(options.get(targetWriteOption) === targetWriteValue)
+    }
+
+    assert(table.lastWriteInfo != null, "the V2 table did not receive LogicalWriteInfo")
+    assert(table.lastWriteInfo.options().get(targetLoadOption) === targetLoadValue)

Review Comment:
   thanks for flagging this! This actually also exposed that `extractTableStateOptions` was iterating `CaseInsensitiveStringMap.entrySet`, which normalizes keys to lowercase, and I changed the projection to iterate `asCaseSensitiveMap()` while retaining case-insensitive matching.



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