liviazhu commented on code in PR #54298:
URL: https://github.com/apache/spark/pull/54298#discussion_r2818760871
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDB.scala:
##########
@@ -981,6 +981,10 @@ class RocksDB(
verifyChangelogRecord(kvVerifier, key, Some(value))
merge(key, value, includesPrefix = useColumnFamilies,
deriveCfName = useColumnFamilies, includesChecksum =
conf.rowChecksumEnabled)
+
+ case RecordType.DELETE_RANGE_RECORD =>
+ // For deleteRange, 'key' is beginKey and 'value' is endKey
Review Comment:
Do we need to `verifyChangelogRecord`?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDBStateStoreProvider.scala:
##########
@@ -1476,12 +1476,18 @@ class RocksDBStateStoreChangeDataReader(
}
}
- val keyRow = currEncoder._1.decodeKey(currRecord._2)
- if (currRecord._3 == null) {
- (currRecord._1, keyRow, null, currentChangelogVersion - 1)
+ if (currRecord._1 == RecordType.DELETE_RANGE_RECORD) {
+ // For delete_range entries, the key and value have different schemas so
we cannot
+ // put endKey into the value field. Leave both key and value as null.
+ (currRecord._1, null, null, currentChangelogVersion - 1)
Review Comment:
We probably need a unit test for this change right?
--
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]
