bereng commented on code in PR #1891:
URL: https://github.com/apache/cassandra/pull/1891#discussion_r1106743032
##########
src/java/org/apache/cassandra/db/rows/Cell.java:
##########
@@ -59,14 +68,24 @@
public interface Factory<V>
{
- Cell<V> create(ColumnMetadata column, long timestamp, int ttl, int
localDeletionTime, V value, CellPath path);
+ Cell<V> create(ColumnMetadata column, long timestamp, int ttl, long
localDeletionTime, V value, CellPath path);
}
protected Cell(ColumnMetadata column)
{
super(column);
}
+ public static int deletionTimeLongToGuavaUint(long deletionTime)
+ {
+ return deletionTime == NO_DELETION_TIME ?
UnsignedInteger.MAX_VALUE.intValue() :
UnsignedInteger.valueOf(deletionTime).intValue();
+ }
+
+ public static long deletionTimeGuavaUintToLong(int deletionTimeGuavaUint)
+ {
+ return deletionTimeGuavaUint == UnsignedInteger.MAX_VALUE.intValue() ?
NO_DELETION_TIME :
UnsignedInteger.fromIntBits(deletionTimeGuavaUint).longValue();
Review Comment:
`UnsignedInts` is sthg I started with. You can see in the trail of commits
it was there. I have a very strong opinionated opposition to using it. This
comes from the experience of working in this PR and the diabolical debug
sessions I had to deal with.
Think that `UnsignedInts` will just swallow any value you throw at it. It
doesn't sanity check anything. The bugs that emerge are just gibberish
timestamps that don't _crash_ the code. You just get silent row removal, silent
row mismatch, etc. Just think in terms of the code randomly changing timestamps
and how this would fan out.
Given the above consider also the future of the codebase. If we sometime
build some new code that accidentally produces an invalid value the bugs that
will emerge from that will be probably silent and 100% sure hard to find. Think
sentinel values, usually 'out of band' values that can easily fall outside a
valid uint range...
It would be very painful for me to move away from `UnsignedInteger` knowing
what I've learnt, it would be very unwise imo and an ugly accident waiting to
happen. I would strongly recommend staying with 'UnsignedInteger'.
--
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]
