aweisberg commented on code in PR #4006: URL: https://github.com/apache/cassandra/pull/4006#discussion_r2019120174
########## src/java/org/apache/cassandra/service/paxos/Paxos.java: ########## @@ -1111,7 +1111,12 @@ private static BeginResult begin(long deadline, PaxosPrepare.Success success = prepare.success(); Supplier<Participants> plan = () -> success.participants; - DataResolver<?, ?> resolver = new DataResolver<>(query, plan, NoopReadRepair.instance, new Dispatcher.RequestTime(query.creationTimeNanos())); + long createdAtNanos = query.creationTimeNanos(); Review Comment: Is `query.creationTimeNanos` the wrong value? Shouldn't we plumb through `Dispatcher.RequestTime` and use it the way we would for normal reads by calling `requestTime.startedAtNanos()` and passing that to `setMonitoringTime`? The real bug here is that it's not called at all for PaxosV2 right? ########## test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java: ########## @@ -333,11 +333,18 @@ protected ConsistencyLevel mutationCl() { var inst = selectInstance(rs); String postfix = "on " + inst; + if (mutation.isCas()) + { + postfix += ", would apply " + model.shouldApply(mutation); + // CAS doesn't allow timestamps Review Comment: CAS doesn't allow timestamps so neither does Accord using the CAS syntax. For non-CAS it does allow timestamps and in fact users coordinator generated timestamps not Accord timestamps at the moment for everything. It would take a little more work to use Accord timestamps where possible with non-SERIAL. ########## src/java/org/apache/cassandra/db/marshal/AbstractType.java: ########## @@ -537,6 +531,22 @@ public boolean allowsEmpty() return false; } + /** + * Returns {@code true} for types where empty should be handled like {@code null} like {@link Int32Type}. + */ + public boolean isEmptyValueMeaningless() Review Comment: Why is this moving? Unnecessary churn? ########## test/unit/org/apache/cassandra/cql3/ast/AssignmentOperator.java: ########## @@ -59,10 +59,15 @@ public static EnumSet<Kind> supportsOperators(AbstractType<?> type, boolean isTr EnumSet<Kind> result = EnumSet.noneOf(Kind.class); if (type instanceof CollectionType && type.isMultiCell()) { - if (type instanceof SetType || type instanceof ListType) + if (type instanceof SetType) return EnumSet.of(Kind.ADD, Kind.SUBTRACT); + if (type instanceof ListType) Review Comment: Is `-=` syntax even allowed with non-SERIAL? I thought it was rejected. -- 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: pr-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org