anishshri-db commented on code in PR #54278:
URL: https://github.com/apache/spark/pull/54278#discussion_r2814899564


##########
sql/core/src/test/scala/org/apache/spark/sql/execution/streaming/state/RocksDBTimestampEncoderOperationsSuite.scala:
##########
@@ -281,6 +281,70 @@ class RocksDBTimestampEncoderOperationsSuite extends 
SharedSparkSession
       }
     }
 
+    test(s"Event time as prefix: iterator with multiple values (encoding = 
$encoding)") {

Review Comment:
   Does Avro work here or will be added later ? if so, can we add a TODO ?



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStore.scala:
##########
@@ -171,10 +171,31 @@ trait ReadStateStore {
       prefixKey: UnsafeRow,
       colFamilyName: String = StateStore.DEFAULT_COL_FAMILY_NAME): 
StateStoreIterator[UnsafeRowPair]
 
+  /**
+   * Return an iterator containing all the (key, value) pairs which are 
matched with
+   * the given prefix key.
+   *
+   * It is expected to throw exception if Spark calls this method without 
proper key encoding spec.
+   * It is also expected to throw exception if Spark calls this method without 
setting
+   * multipleValuesPerKey as true for the column family.

Review Comment:
   Where do we verify the `multipleValuesPerKey` condition ?



##########
sql/core/src/test/scala/org/apache/spark/sql/execution/streaming/state/RocksDBTimestampEncoderOperationsSuite.scala:
##########
@@ -281,6 +281,70 @@ class RocksDBTimestampEncoderOperationsSuite extends 
SharedSparkSession
       }
     }
 
+    test(s"Event time as prefix: iterator with multiple values (encoding = 
$encoding)") {
+      tryWithProviderResource(
+        newStoreProviderWithTimestampEncoder(
+          encoderType = "prefix",
+          useMultipleValuesPerKey = true,
+          dataEncoding = encoding)
+      ) { provider =>
+        val store = provider.getStore(0)
+
+        try {
+          // Put multiple values at different event times
+          // Insert in non-sorted order to verify ordering by event time
+          val values2 = Array(valueToRow(200), valueToRow(201))
+          store.putList(keyAndTimestampToRow("key1", 1, 1000L), values2)

Review Comment:
   Lets add some different values here also ? similar to the encoder tests ?



##########
sql/core/src/test/scala/org/apache/spark/sql/execution/streaming/state/RocksDBTimestampEncoderOperationsSuite.scala:
##########
@@ -380,16 +528,18 @@ class RocksDBTimestampEncoderOperationsSuite extends 
SharedSparkSession
         val iter = encoderType match {
           // For prefix encoder, we use iterator
           case "prefix" =>
-            store.iterator()
+            if (useMultipleValuesPerKey) store.iteratorWithMultiValues()
+            else store.iterator()
           // For postfix encoder, we use prefix scan with ("key1", 1) as the 
prefix key
           case "postfix" =>
-            store.prefixScan(keyToRow("key1", 1))
+            if (useMultipleValuesPerKey) 
store.prefixScanWithMultiValues(keyToRow("key1", 1))

Review Comment:
   same here ?



##########
sql/core/src/test/scala/org/apache/spark/sql/execution/streaming/state/RocksDBTimestampEncoderOperationsSuite.scala:
##########
@@ -380,16 +528,18 @@ class RocksDBTimestampEncoderOperationsSuite extends 
SharedSparkSession
         val iter = encoderType match {
           // For prefix encoder, we use iterator
           case "prefix" =>
-            store.iterator()
+            if (useMultipleValuesPerKey) store.iteratorWithMultiValues()

Review Comment:
   Could we use braces ?



-- 
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: [email protected]

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