maedhroz commented on code in PR #4748:
URL: https://github.com/apache/cassandra/pull/4748#discussion_r3102685003


##########
src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java:
##########
@@ -554,21 +554,26 @@ public void writeRangeTombstone(UnfilteredDescriptor 
rangeTombstone, boolean upd
         updateMetadataAndIndexBlock(rangeTombstone, unfilteredStartPosition, 
unfilteredEndPosition, updateClusteringMetadata);
     }
 
-    private void updateMetadataAndIndexBlock(
-        UnfilteredDescriptor unfilteredDescriptor,
-        long unfilteredStartPosition,
-        long unfilteredEndPosition,
-        boolean updateClusteringMetadata) throws IOException
+    private void updateMetadataAndIndexBlock(UnfilteredDescriptor 
unfilteredDescriptor,
+                                             long unfilteredStartPosition,
+                                             long unfilteredEndPosition,
+                                             boolean updateClusteringMetadata) 
throws IOException
     {
         if (updateClusteringMetadata) 
updateClusteringMetadata(unfilteredDescriptor);
         // write the first clustering into rowIndexEntries buffer (we will 
need it unless we never write the first entry)
-        if (unfilteredStartPosition == indexBlockStartOffset || 
(rowIndexEntryOffset == rowIndexEntries.position())) {
+        if (currentOffsetInPartition(unfilteredStartPosition) == 
indexBlockStartOffset || (rowIndexEntryOffset == rowIndexEntries.position()))
+        {
             writeClusteringToRowIndexEntries(unfilteredDescriptor);
         }
         else
         {
-            rowIndexEntryLastClustering.copy(unfilteredDescriptor);
+            // Only update last clustering tracking if the descriptor has 
actual bytes. A synthesized end-of-partition 
+            // bound (e.g. INCL_END_BOUND with clusteringLength == 0) carries 
no real position and should not overwrite 
+            // the last real clustering.
+            if (unfilteredDescriptor.clusteringLength() > 0)
+                rowIndexEntryLastClustering.copy(unfilteredDescriptor);
         }

Review Comment:
   Yeah, even though this passes the test, I think it is actually causing a 
different last clustering to be written in the `IndexInfo`. Looking at another 
angle where I fix in `addIndexBlock()` instead...



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