diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitMutationComposer.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitMutationComposer.java index ae1da0e428..71042a7210 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitMutationComposer.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitMutationComposer.java @@ -9,6 +9,7 @@ import com.scalar.db.api.ConditionBuilder; import com.scalar.db.api.Consistency; import com.scalar.db.api.Delete; +import com.scalar.db.api.DeleteBuilder; import com.scalar.db.api.Mutation; import com.scalar.db.api.Operation; import com.scalar.db.api.Put; @@ -114,17 +115,20 @@ private Put composePut(Operation base, @Nullable TransactionResult result) private Delete composeDelete(Operation base, @Nullable TransactionResult result) throws ExecutionException { - return Delete.newBuilder() - .namespace(base.forNamespace().get()) - .table(base.forTable().get()) - .partitionKey(getPartitionKey(base, result)) - .clusteringKey(getClusteringKey(base, result).orElse(null)) - .consistency(Consistency.LINEARIZABLE) - .condition( - ConditionBuilder.deleteIf(ConditionBuilder.column(ID).isEqualToText(id)) - .and(ConditionBuilder.column(STATE).isEqualToInt(TransactionState.DELETED.get())) - .build()) - .build(); + DeleteBuilder.Buildable deleteBuilder = + Delete.newBuilder() + .namespace(base.forNamespace().get()) + .table(base.forTable().get()) + .partitionKey(getPartitionKey(base, result)) + .consistency(Consistency.LINEARIZABLE) + .condition( + ConditionBuilder.deleteIf(ConditionBuilder.column(ID).isEqualToText(id)) + .and( + ConditionBuilder.column(STATE).isEqualToInt(TransactionState.DELETED.get())) + .build()); + getClusteringKey(base, result).ifPresent(deleteBuilder::clusteringKey); + + return deleteBuilder.build(); } private Key getPartitionKey(Operation base, @Nullable TransactionResult result) diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/RollbackMutationComposer.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/RollbackMutationComposer.java index bc70497476..f1fe07d64f 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/RollbackMutationComposer.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/RollbackMutationComposer.java @@ -8,8 +8,10 @@ import com.scalar.db.api.ConditionBuilder; import com.scalar.db.api.Consistency; import com.scalar.db.api.Delete; +import com.scalar.db.api.DeleteBuilder; import com.scalar.db.api.DistributedStorage; import com.scalar.db.api.Get; +import com.scalar.db.api.GetBuilder; import com.scalar.db.api.Mutation; import com.scalar.db.api.Operation; import com.scalar.db.api.Put; @@ -121,17 +123,19 @@ private Delete composeDelete(Operation base, TransactionResult result) throws Ex Key partitionKey = ScalarDbUtils.getPartitionKey(result, tableMetadata); Optional clusteringKey = ScalarDbUtils.getClusteringKey(result, tableMetadata); - return Delete.newBuilder() - .namespace(base.forNamespace().get()) - .table(base.forTable().get()) - .partitionKey(partitionKey) - .clusteringKey(clusteringKey.orElse(null)) - .condition( - ConditionBuilder.deleteIf(ConditionBuilder.column(ID).isEqualToText(id)) - .and(ConditionBuilder.column(STATE).isEqualToInt(result.getState().get())) - .build()) - .consistency(Consistency.LINEARIZABLE) - .build(); + DeleteBuilder.Buildable deleteBuilder = + Delete.newBuilder() + .namespace(base.forNamespace().get()) + .table(base.forTable().get()) + .partitionKey(partitionKey) + .condition( + ConditionBuilder.deleteIf(ConditionBuilder.column(ID).isEqualToText(id)) + .and(ConditionBuilder.column(STATE).isEqualToInt(result.getState().get())) + .build()) + .consistency(Consistency.LINEARIZABLE); + clusteringKey.ifPresent(deleteBuilder::clusteringKey); + + return deleteBuilder.build(); } private Optional getLatestResult( @@ -157,15 +161,16 @@ private Optional getLatestResult( } } - Get get = + GetBuilder.BuildableGetWithPartitionKey getBuilder = Get.newBuilder() .namespace(operation.forNamespace().get()) .table(operation.forTable().get()) .partitionKey(partitionKey) - .clusteringKey(clusteringKey) - .consistency(Consistency.LINEARIZABLE) - .build(); + .consistency(Consistency.LINEARIZABLE); + if (clusteringKey != null) { + getBuilder.clusteringKey(clusteringKey); + } - return storage.get(get).map(TransactionResult::new); + return storage.get(getBuilder.build()).map(TransactionResult::new); } } diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitMutationComposerTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitMutationComposerTest.java index c6fe6dbc83..fb3c041403 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitMutationComposerTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitMutationComposerTest.java @@ -118,6 +118,25 @@ private Delete prepareDelete() { .build(); } + private Put preparePutWithoutCk() { + Key partitionKey = Key.ofText(ANY_NAME_1, ANY_TEXT_1); + return Put.newBuilder() + .namespace(ANY_NAMESPACE_NAME) + .table(ANY_TABLE_NAME) + .partitionKey(partitionKey) + .intValue(ANY_NAME_3, ANY_INT_1) + .build(); + } + + private Delete prepareDeleteWithoutCk() { + Key partitionKey = Key.ofText(ANY_NAME_1, ANY_TEXT_1); + return Delete.newBuilder() + .namespace(ANY_NAMESPACE_NAME) + .table(ANY_TABLE_NAME) + .partitionKey(partitionKey) + .build(); + } + private Get prepareGet() { Key partitionKey = Key.ofText(ANY_NAME_1, ANY_TEXT_1); Key clusteringKey = Key.ofText(ANY_NAME_2, ANY_TEXT_2); @@ -220,6 +239,52 @@ public void add_PutAndNullResultGiven_ShouldComposePutWithPutIfCondition() assertThat(actual).isEqualTo(expected); } + @Test + public void add_PutWithoutCkAndPreparedResultGiven_ShouldComposePutWithPutIfCondition() + throws ExecutionException { + // Arrange + Put put = preparePutWithoutCk(); + TransactionResult result = prepareResult(TransactionState.PREPARED); + + // Act + composer.add(put, result); + + // Assert + Put actual = (Put) composer.get().get(0); + Put expected = + Put.newBuilder() + .namespace(put.forNamespace().get()) + .table(put.forTable().get()) + .partitionKey(put.getPartitionKey()) + .consistency(Consistency.LINEARIZABLE) + .condition( + ConditionBuilder.putIf(ConditionBuilder.column(ID).isEqualToText(ANY_ID)) + .and( + ConditionBuilder.column(STATE) + .isEqualToInt(TransactionState.PREPARED.get())) + .build()) + .bigIntValue(COMMITTED_AT, ANY_TIME_2) + .intValue(STATE, TransactionState.COMMITTED.get()) + .textValue(BEFORE_ID, null) + .intValue(BEFORE_STATE, null) + .intValue(BEFORE_VERSION, null) + .bigIntValue(BEFORE_PREPARED_AT, null) + .bigIntValue(BEFORE_COMMITTED_AT, null) + .intValue(BEFORE_PREFIX + ANY_NAME_3, null) + .booleanValue(BEFORE_PREFIX + ANY_NAME_4, null) + .bigIntValue(BEFORE_PREFIX + ANY_NAME_5, null) + .floatValue(BEFORE_PREFIX + ANY_NAME_6, null) + .doubleValue(BEFORE_PREFIX + ANY_NAME_7, null) + .textValue(BEFORE_PREFIX + ANY_NAME_8, null) + .blobValue(BEFORE_PREFIX + ANY_NAME_9, (byte[]) null) + .dateValue(BEFORE_PREFIX + ANY_NAME_10, null) + .timeValue(BEFORE_PREFIX + ANY_NAME_11, null) + .timestampValue(BEFORE_PREFIX + ANY_NAME_12, null) + .timestampTZValue(BEFORE_PREFIX + ANY_NAME_13, null) + .build(); + assertThat(actual).isEqualTo(expected); + } + @Test public void add_DeleteAndDeletedResultGiven_ShouldComposeDeleteWithDeleteIfCondition() throws ExecutionException { @@ -267,6 +332,30 @@ public void add_DeleteAndNullResultGiven_ShouldComposeDeleteWithDeleteIfConditio assertThat(actual).isEqualTo(delete); } + @Test + public void add_DeleteWithoutCkAndDeletedResultGiven_ShouldComposeDeleteWithDeleteIfCondition() + throws ExecutionException { + // Arrange + Delete delete = prepareDeleteWithoutCk(); + TransactionResult result = prepareResult(TransactionState.DELETED); + + // Act + composer.add(delete, result); + + // Assert + Delete actual = (Delete) composer.get().get(0); + delete = + Delete.newBuilder(delete) + .consistency(Consistency.LINEARIZABLE) + .condition( + ConditionBuilder.deleteIf(ConditionBuilder.column(ID).isEqualToText(ANY_ID)) + .and( + ConditionBuilder.column(STATE).isEqualToInt(TransactionState.DELETED.get())) + .build()) + .build(); + assertThat(actual).isEqualTo(delete); + } + @Test public void add_SelectionAndPreparedResultGiven_ShouldComposePutForRollforward() throws ExecutionException { diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/RollbackMutationComposerTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/RollbackMutationComposerTest.java index 6c554e57d1..21270a5d55 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/RollbackMutationComposerTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/RollbackMutationComposerTest.java @@ -188,6 +188,25 @@ private Put preparePut() { .build(); } + private Put preparePutWithoutCk() { + return Put.newBuilder() + .namespace(ANY_NAMESPACE_NAME) + .table(ANY_TABLE_NAME) + .partitionKey(Key.ofText(ANY_NAME_1, ANY_TEXT_1)) + .intValue(ANY_NAME_3, ANY_INT_3) + .booleanValue(ANY_NAME_4, false) + .bigIntValue(ANY_NAME_5, ANY_BIGINT_3) + .floatValue(ANY_NAME_6, ANY_FLOAT_3) + .doubleValue(ANY_NAME_7, ANY_DOUBLE_3) + .textValue(ANY_NAME_8, ANY_TEXT_4) + .blobValue(ANY_NAME_9, ANY_BLOB_3) + .dateValue(ANY_NAME_10, ANY_DATE_3) + .timeValue(ANY_NAME_11, ANY_TIME_3) + .timestampValue(ANY_NAME_12, ANY_TIMESTAMP_3) + .timestampTZValue(ANY_NAME_13, ANY_TIMESTAMPTZ_3) + .build(); + } + private TransactionResult prepareResult(TransactionState state) { ImmutableMap> columns = ImmutableMap.>builder() @@ -548,6 +567,22 @@ public void add_PutAndNullResultGivenAndOldResultGivenFromStorage_ShouldDoNothin verify(storage).get(any(Get.class)); } + @Test + public void add_PutWithoutCkAndNullResultGivenAndOldResultGivenFromStorage_ShouldDoNothing() + throws ExecutionException { + // Arrange + TransactionResult result = prepareInitialResult(ANY_ID_1, TransactionState.PREPARED); + when(storage.get(any(Get.class))).thenReturn(Optional.of(result)); + Put put = preparePutWithoutCk(); + + // Act + composer.add(put, null); + + // Assert + assertThat(composer.get().size()).isEqualTo(0); + verify(storage).get(any(Get.class)); + } + @Test public void add_PutAndNullResultGivenAndEmptyResultGivenFromStorage_ShouldDoNothing() throws ExecutionException {