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