dcapwell commented on code in PR #4220:
URL: https://github.com/apache/cassandra/pull/4220#discussion_r2240894280


##########
test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java:
##########
@@ -206,10 +211,55 @@ protected <S extends BaseState> 
Property.StatefulSuccess<S, Void> onSuccess(Logg
         });
     }
 
+    protected static <S extends CommonState> Property.Command<S, Void, ?> 
validateUsingTimestamp(RandomSource rs, S state)
+    {
+        if (state.operations == 0)
+            return ignoreCommand();
+        var builder = Select.builder(state.metadata);
+        for (var c : state.model.factory.regularAndStaticColumns)
+            builder.selection(FunctionCall.writetime(c));
+        ByteBuffer upperboundTimestamp = LongType.instance.decompose((long) 
state.operations);
+        var select = builder.build();
+        var inst = state.selectInstance(rs);
+        return new Property.SimpleCommand<>(state.humanReadable(select, null), 
s -> {
+            var result = s.executeQuery(inst, Integer.MAX_VALUE, s.selectCl(), 
select);
+            for (var row : result)
+            {
+                for (var col : state.model.factory.regularAndStaticColumns)
+                {
+                    int idx = 
state.model.factory.regularAndStaticColumns.indexOf(col);
+                    ByteBuffer value = row[idx];
+                    if (value == null) continue;
+                    if (col.type().isMultiCell())
+                    {
+                        List<ByteBuffer> timestamps = 
LONG_LIST_TYPE.unpack(value);
+                        int cellIndex = 0;
+                        for (var timestamp : timestamps)
+                        {
+                            
Assertions.assertThat(LongType.instance.compare(timestamp, upperboundTimestamp))
+                                      .describedAs("Unexected timestamp at 
multi-cell index %s for col %s: %s > %s", cellIndex, col, 
LongType.instance.compose(timestamp), state.operations)
+                                      .isLessThanOrEqualTo(state.operations);
+                            cellIndex++;
+                        }
+                    }
+                    else
+                    {
+                        Assertions.assertThat(LongType.instance.compare(value, 
upperboundTimestamp))
+                                  .describedAs("Unexected timestamp for col 
%s: %s > %s", col, LongType.instance.compose(value), state.operations)
+                                  .isLessThanOrEqualTo(state.operations);
+                    }
+                }
+            }
+        });
+    }
+
     protected static <S extends CommonState> Property.Command<S, Void, ?> 
insert(RandomSource rs, S state)
     {
         int timestamp = ++state.operations;
-        Mutation mutation = 
state.mutationGen().next(rs).withTimestamp(timestamp);
+        Mutation original = state.mutationGen().next(rs);
+        Mutation mutation = state.allowUsingTimestamp()
+                            ? original.withTimestamp(timestamp)

Review Comment:
   > but doesn't actually work correctly for real world unlogged single 
partition batches which can have different rows with different timestamps.
   
   The test isn't trying to cover that, the whole `USING TIMESTAMP` logic is to 
make sure that the test is strict serializable else the model fails.  When we 
run on accord the idea is that we no longer need to use `USING TIMESTAMP` as 
Accord makes sure this is safe.



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

Reply via email to