aweisberg commented on code in PR #3777:
URL: https://github.com/apache/cassandra/pull/3777#discussion_r1917276338
##########
src/java/org/apache/cassandra/service/accord/txn/AbstractKeySorted.java:
##########
@@ -51,17 +58,63 @@ public AbstractKeySorted(List<T> items)
{
T[] arr = newArray(items.size());
items.toArray(arr);
- Arrays.sort(arr, this::compare);
this.items = arr;
+ if (items.size() == 0)
+ {
+ this.itemKeys = Keys.of();
+ return;
+ }
+ Domain domain = getKeys(arr[0]).domain();
+ switch (domain)
+ {
+ case Key:
+ Arrays.sort(arr, this::compareKey);
+ break;
+ case Range:
+ Arrays.sort(arr, this::compareRange);
+ break;
+ default:
+ throw new IllegalStateException("Unhandled domain " + domain);
+ }
this.itemKeys = extractItemKeys();
}
- private Keys extractItemKeys()
+ private Seekables extractItemKeys()
{
- PartitionKey[] keys = new PartitionKey[items.length];
- for (int i = 0 ; i < keys.length ; ++i)
- keys[i] = getKey(items[i]);
- return Keys.ofSorted(keys);
+ // TODO (review): This doesn't pick the "right" domain which could
make Accord angry (in practice it doesn't)
+ // but I think the right track going forward is to be selectively
forgiving of empty Seekables in the wrong
+ // domain in Accord for `Update.key()` and `Read.keys` to save
implementations from having to propagate them
+ // or alternatively just allow null `Read` just like we allow null
`Query` and null `Update`
+ if (items.length == 0)
+ return Keys.of();
Review Comment:
`TxnRead.EMPTY` is not a fine thing is the point. You will have `Txn` where
`Txn.keys()` is one domain and `Txn.read()` is a different domain (potentially
from `Txn.write()`). `Txn.write()` is allowed to return null so you don't have
to worry about finding and supplying the correct domain.
Does this impact Accord today? No. Could it cause a bug in the future? Maybe?
--
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]
