Fix secondary index behavior in Consensus Commit (#3133)

Author: brfrn169

core/src/integration-test/java/com/scalar/db/storage/objectstorage/ConsensusCommitSpecificIntegrationTestWithObjectStorage.java

@@ -59,80 +59,11 @@ protected Properties getProperties(String testName) {
 
   @Override
   @Disabled("Object Storage does not support index-related operations")
-  public void scan_ScanWithIndexGiven_WithSerializable_ShouldNotThrowAnyException() {}
+  public void scan_ScanWithIndexGiven_WithSerializable_ShouldThrowIllegalArgumentException() {}
 
   @Override
   @Disabled("Object Storage does not support index-related operations")
-  public void
-      scan_ScanWithIndexGiven_RecordUpdatedByAnotherTransaction_WithSerializable_ShouldThrowCommitConflictException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      scan_ScanWithIndexGiven_RecordUpdatedByMyself_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      scan_ScanWithIndexGiven_RecordDeletedByAnotherTransaction_WithSerializable_ShouldThrowCommitConflictException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      scan_ScanWithIndexGiven_RecordDeletedByMyself_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void scan_ScanWithIndexWithLimitGiven_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void get_GetWithIndexGiven_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_RecordUpdatedByAnotherTransaction_WithSerializable_ShouldThrowCommitConflictException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_RecordUpdatedByMyself_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_RecordDeletedByAnotherTransaction_WithSerializable_ShouldThrowCommitConflictException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_RecordDeletedByMyself_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_NoRecordsInIndexRange_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_RecordInsertedIntoIndexRangeByMyself_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_RecordInsertedIntoIndexRangeByAnotherTransaction_WithSerializable_ShouldThrowCommitConflictException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_NoRecordsInIndexRange_RecordInsertedIntoIndexRangeByMyself_WithSerializable_ShouldNotThrowAnyException() {}
-
-  @Override
-  @Disabled("Object Storage does not support index-related operations")
-  public void
-      get_GetWithIndexGiven_NoRecordsInIndexRange_RecordInsertedIntoIndexRangeByAnotherTransaction_WithSerializable_ShouldThrowCommitConflictException() {}
+  public void get_GetWithIndexGiven_WithSerializable_ShouldThrowIllegalArgumentException() {}
 
   @Override
   @Disabled("Object Storage does not support index-related operations")

core/src/main/java/com/scalar/db/common/CoreError.java

@@ -901,6 +901,36 @@ public enum CoreError implements ScalarDbError {
       "The value of the column %s in the primary key contains an illegal character.",
       "",
       ""),
+  CONSENSUS_COMMIT_SPECIFYING_TRANSACTION_METADATA_COLUMNS_IN_PROJECTION_NOT_ALLOWED(
+      Category.USER_ERROR,
+      "0258",
+      "Specifying transaction metadata columns in the projection is not allowed. Table: %s; Column: %s",
+      "",
+      ""),
+  CONSENSUS_COMMIT_SPECIFYING_TRANSACTION_METADATA_COLUMNS_IN_ORDERING_NOT_ALLOWED(
+      Category.USER_ERROR,
+      "0259",
+      "Specifying transaction metadata columns in the ordering is not allowed. Table: %s; Column: %s",
+      "",
+      ""),
+  CONSENSUS_COMMIT_INDEX_GET_NOT_ALLOWED_IN_SERIALIZABLE(
+      Category.USER_ERROR,
+      "0260",
+      "Get operations by using an index is not allowed in the SERIALIZABLE isolation level",
+      "",
+      ""),
+  CONSENSUS_COMMIT_INDEX_SCAN_NOT_ALLOWED_IN_SERIALIZABLE(
+      Category.USER_ERROR,
+      "0261",
+      "Scan operations by using an index is not allowed in the SERIALIZABLE isolation level",
+      "",
+      ""),
+  CONSENSUS_COMMIT_CONDITION_ON_INDEXED_COLUMNS_NOT_ALLOWED_IN_CROSS_PARTITION_SCAN_IN_SERIALIZABLE(
+      Category.USER_ERROR,
+      "0262",
+      "Conditions on indexed columns in cross-partition scan operations are not allowed in the SERIALIZABLE isolation level",
+      "",
+      ""),
 
   //
   // Errors for the concurrency error category

core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommit.java

@@ -50,20 +50,20 @@ public class ConsensusCommit extends AbstractDistributedTransaction {
   private final TransactionContext context;
   private final CrudHandler crud;
   private final CommitHandler commit;
-  private final ConsensusCommitMutationOperationChecker mutationOperationChecker;
+  private final ConsensusCommitOperationChecker operationChecker;
   @Nullable private final CoordinatorGroupCommitter groupCommitter;
 
   @SuppressFBWarnings("EI_EXPOSE_REP2")
   public ConsensusCommit(
       TransactionContext context,
       CrudHandler crud,
       CommitHandler commit,
-      ConsensusCommitMutationOperationChecker mutationOperationChecker,
+      ConsensusCommitOperationChecker operationChecker,
       @Nullable CoordinatorGroupCommitter groupCommitter) {
     this.context = checkNotNull(context);
     this.crud = checkNotNull(crud);
     this.commit = checkNotNull(commit);
-    this.mutationOperationChecker = mutationOperationChecker;
+    this.operationChecker = operationChecker;
     this.groupCommitter = groupCommitter;
   }
 
@@ -74,17 +74,40 @@ public String getId() {
 
   @Override
   public Optional<Result> get(Get get) throws CrudException {
-    return crud.get(copyAndSetTargetToIfNot(get), context);
+    get = copyAndSetTargetToIfNot(get);
+
+    try {
+      operationChecker.check(get, context);
+    } catch (ExecutionException e) {
+      throw new CrudException(e.getMessage(), e, getId());
+    }
+
+    return crud.get(get, context);
   }
 
   @Override
   public List<Result> scan(Scan scan) throws CrudException {
-    return crud.scan(copyAndSetTargetToIfNot(scan), context);
+    scan = copyAndSetTargetToIfNot(scan);
+
+    try {
+      operationChecker.check(scan, context);
+    } catch (ExecutionException e) {
+      throw new CrudException(e.getMessage(), e, getId());
+    }
+
+    return crud.scan(scan, context);
   }
 
   @Override
   public Scanner getScanner(Scan scan) throws CrudException {
     scan = copyAndSetTargetToIfNot(scan);
+
+    try {
+      operationChecker.check(scan, context);
+    } catch (ExecutionException e) {
+      throw new CrudException(e.getMessage(), e, getId());
+    }
+
     return crud.getScanner(scan, context);
   }
 
@@ -230,7 +253,7 @@ void waitForRecoveryCompletion() throws CrudException {
 
   private void checkMutation(Mutation mutation) throws CrudException {
     try {
-      mutationOperationChecker.check(mutation);
+      operationChecker.check(mutation);
     } catch (ExecutionException e) {
       throw new CrudException(e.getMessage(), e, getId());
     }

core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java

@@ -58,7 +58,7 @@ public class ConsensusCommitManager extends AbstractDistributedTransactionManage
   private final CrudHandler crud;
   protected final CommitHandler commit;
   private final Isolation isolation;
-  private final ConsensusCommitMutationOperationChecker mutationOperationChecker;
+  private final ConsensusCommitOperationChecker operationChecker;
   @Nullable private final CoordinatorGroupCommitter groupCommitter;
 
   @SuppressFBWarnings("EI_EXPOSE_REP2")
@@ -86,7 +86,9 @@ public ConsensusCommitManager(
             parallelExecutor);
     commit = createCommitHandler(config);
     isolation = config.getIsolation();
-    mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager);
+    operationChecker =
+        new ConsensusCommitOperationChecker(
+            tableMetadataManager, config.isIncludeMetadataEnabled());
   }
 
   protected ConsensusCommitManager(DatabaseConfig databaseConfig) {
@@ -113,14 +115,17 @@ protected ConsensusCommitManager(DatabaseConfig databaseConfig) {
             parallelExecutor);
     commit = createCommitHandler(config);
     isolation = config.getIsolation();
-    mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager);
+    operationChecker =
+        new ConsensusCommitOperationChecker(
+            tableMetadataManager, config.isIncludeMetadataEnabled());
   }
 
   @SuppressFBWarnings("EI_EXPOSE_REP2")
   @VisibleForTesting
   ConsensusCommitManager(
       DistributedStorage storage,
       DistributedStorageAdmin admin,
+      ConsensusCommitConfig config,
       DatabaseConfig databaseConfig,
       Coordinator coordinator,
       ParallelExecutor parallelExecutor,
@@ -142,8 +147,9 @@ protected ConsensusCommitManager(DatabaseConfig databaseConfig) {
     this.commit = commit;
     this.groupCommitter = groupCommitter;
     this.isolation = isolation;
-    this.mutationOperationChecker =
-        new ConsensusCommitMutationOperationChecker(tableMetadataManager);
+    this.operationChecker =
+        new ConsensusCommitOperationChecker(
+            tableMetadataManager, config.isIncludeMetadataEnabled());
   }
 
   // `groupCommitter` must be set before calling this method.
@@ -261,7 +267,7 @@ DistributedTransaction begin(
     TransactionContext context =
         new TransactionContext(txId, snapshot, isolation, readOnly, oneOperation);
     DistributedTransaction transaction =
-        new ConsensusCommit(context, crud, commit, mutationOperationChecker, groupCommitter);
+        new ConsensusCommit(context, crud, commit, operationChecker, groupCommitter);
     if (readOnly) {
       transaction = new ReadOnlyDistributedTransaction(transaction);
     }