HeartSaVioR commented on code in PR #45503:
URL: https://github.com/apache/spark/pull/45503#discussion_r1536565745


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDBStateEncoder.scala:
##########
@@ -212,6 +212,12 @@ class PrefixKeyScanStateEncoder(
  * We cannot support variable sized fields given the UnsafeRow format which 
stores variable
  * sized fields as offset and length pointers to the actual values, thereby 
changing the required
  * ordering.
+ * Note that we also support "null" values being passed for these fixed size 
fields. We prepend
+ * a single byte to indicate whether the column value is null or not. We 
cannot change the
+ * nullability on the UnsafeRow itself as the expected ordering would change 
if non-first
+ * columns are marked as null. If the first col is null, those entries will 
appear last in
+ * the iterator. If non-first columns are null, ordering based on the previous 
columns will

Review Comment:
   This comment actually confused me. If I'm not missing here, it's just 
applying "null-last" for each column, and ordering should just work as the 
same. E.g. `(1, 1, 3, 1, 2), (1, 1, null, 1, 3)`, `(1, 1, null, 2, 2)` should 
be still ordered. Having null in the middle of column does not block the data 
to be ordered for remaining columns.
   
   Please correct me if I'm missing something. If I'm on the right track, could 
we please update the comment and also add a test to check this behavior? 
Probably adding more values in the latest test case would work.



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