korlov42 commented on code in PR #7936:
URL: https://github.com/apache/ignite-3/pull/7936#discussion_r3043496614
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/TransactionEnlistTest.java:
##########
@@ -104,6 +152,25 @@ void testEnlistCall() {
Mockito.verify(spiedTx, times(2)).enlist(any(), anyInt(), any(),
anyLong());
}
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2, 3})
+ void testCommitPartitionChoice(int id) {
+ NoOpTransaction tx = NoOpTransaction.readWrite("t1", false);
+
+ NoOpTransaction spiedTx = Mockito.spy(tx);
+
+ assertQuery("UPDATE empty_table_for_update SET val = 42 WHERE id = ?",
spiedTx)
+ .withParam(id)
+ .check();
+
+ int expectedPartition = expectedPartition(id);
+ ArgumentMatcher<ZonePartitionId> partitionIdMatcher = zonePartitionId
-> zonePartitionId.partitionId() == expectedPartition;
+ Mockito.verify(spiedTx, times(2))
+ .assignCommitPartition(argThat(partitionIdMatcher));
+ Mockito.verify(spiedTx, times(2))
Review Comment:
> Didn't catch why do we expect times(2)
we use to have tree visitor which does enlistment individually for every
table-containing rel (such as `IndexScan`, `TableScan`, and `TableModify`). For
every such rel, it assigns commit partition and then enlist all involved
partitions.
But after your comment I decided to rework this part. Apparently we have all
required info into the fragment descriptor, so we don't need to traverse the
tree. Also, I made assignment of commit partition optional: we will try to
assign only if transaction has no commit partition at all.
--
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]
