Skip to content

Commit e07c4ea

Browse files
committed
Fix secondary index behavior
1 parent 477fba5 commit e07c4ea

21 files changed

+1452
-1275
lines changed

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,36 @@ public enum CoreError implements ScalarDbError {
889889
"Object Storage does not support the feature for altering column types",
890890
"",
891891
""),
892+
CONSENSUS_COMMIT_SPECIFYING_TRANSACTION_METADATA_COLUMNS_IN_PROJECTION_NOT_ALLOWED(
893+
Category.USER_ERROR,
894+
"0256",
895+
"Specifying transaction metadata columns in the projection is not allowed. Table: %s; Column: %s",
896+
"",
897+
""),
898+
CONSENSUS_COMMIT_SPECIFYING_TRANSACTION_METADATA_COLUMNS_IN_ORDERING_NOT_ALLOWED(
899+
Category.USER_ERROR,
900+
"0257",
901+
"Specifying transaction metadata columns in the ordering is not allowed. Table: %s; Column: %s",
902+
"",
903+
""),
904+
CONSENSUS_COMMIT_INDEX_GET_NOT_ALLOWED_IN_SERIALIZABLE(
905+
Category.USER_ERROR,
906+
"0258",
907+
"Get operations using an index is not allowed in the SERIALIZABLE isolation level",
908+
"",
909+
""),
910+
CONSENSUS_COMMIT_INDEX_SCAN_NOT_ALLOWED_IN_SERIALIZABLE(
911+
Category.USER_ERROR,
912+
"0259",
913+
"Scan operations using an index is not allowed in the SERIALIZABLE isolation level",
914+
"",
915+
""),
916+
CONSENSUS_COMMIT_CONDITION_ON_INDEXED_COLUMNS_NOT_ALLOWED_IN_CROSS_PARTITION_SCAN_IN_SERIALIZABLE(
917+
Category.USER_ERROR,
918+
"0260",
919+
"Conditions on indexed columns in cross-partition scan operations are not allowed in the SERIALIZABLE isolation level",
920+
"",
921+
""),
892922

893923
//
894924
// Errors for the concurrency error category

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

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,20 @@ public class ConsensusCommit extends AbstractDistributedTransaction {
5050
private final TransactionContext context;
5151
private final CrudHandler crud;
5252
private final CommitHandler commit;
53-
private final ConsensusCommitMutationOperationChecker mutationOperationChecker;
53+
private final ConsensusCommitOperationChecker operationChecker;
5454
@Nullable private final CoordinatorGroupCommitter groupCommitter;
5555

5656
@SuppressFBWarnings("EI_EXPOSE_REP2")
5757
public ConsensusCommit(
5858
TransactionContext context,
5959
CrudHandler crud,
6060
CommitHandler commit,
61-
ConsensusCommitMutationOperationChecker mutationOperationChecker,
61+
ConsensusCommitOperationChecker operationChecker,
6262
@Nullable CoordinatorGroupCommitter groupCommitter) {
6363
this.context = checkNotNull(context);
6464
this.crud = checkNotNull(crud);
6565
this.commit = checkNotNull(commit);
66-
this.mutationOperationChecker = mutationOperationChecker;
66+
this.operationChecker = operationChecker;
6767
this.groupCommitter = groupCommitter;
6868
}
6969

@@ -74,17 +74,40 @@ public String getId() {
7474

7575
@Override
7676
public Optional<Result> get(Get get) throws CrudException {
77-
return crud.get(copyAndSetTargetToIfNot(get), context);
77+
get = copyAndSetTargetToIfNot(get);
78+
79+
try {
80+
operationChecker.check(get, context);
81+
} catch (ExecutionException e) {
82+
throw new CrudException(e.getMessage(), e, getId());
83+
}
84+
85+
return crud.get(get, context);
7886
}
7987

8088
@Override
8189
public List<Result> scan(Scan scan) throws CrudException {
82-
return crud.scan(copyAndSetTargetToIfNot(scan), context);
90+
scan = copyAndSetTargetToIfNot(scan);
91+
92+
try {
93+
operationChecker.check(scan, context);
94+
} catch (ExecutionException e) {
95+
throw new CrudException(e.getMessage(), e, getId());
96+
}
97+
98+
return crud.scan(scan, context);
8399
}
84100

85101
@Override
86102
public Scanner getScanner(Scan scan) throws CrudException {
87103
scan = copyAndSetTargetToIfNot(scan);
104+
105+
try {
106+
operationChecker.check(scan, context);
107+
} catch (ExecutionException e) {
108+
throw new CrudException(e.getMessage(), e, getId());
109+
}
110+
88111
return crud.getScanner(scan, context);
89112
}
90113

@@ -230,7 +253,7 @@ void waitForRecoveryCompletion() throws CrudException {
230253

231254
private void checkMutation(Mutation mutation) throws CrudException {
232255
try {
233-
mutationOperationChecker.check(mutation);
256+
operationChecker.check(mutation);
234257
} catch (ExecutionException e) {
235258
throw new CrudException(e.getMessage(), e, getId());
236259
}

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class ConsensusCommitManager extends AbstractDistributedTransactionManage
5858
private final CrudHandler crud;
5959
protected final CommitHandler commit;
6060
private final Isolation isolation;
61-
private final ConsensusCommitMutationOperationChecker mutationOperationChecker;
61+
private final ConsensusCommitOperationChecker operationChecker;
6262
@Nullable private final CoordinatorGroupCommitter groupCommitter;
6363

6464
@SuppressFBWarnings("EI_EXPOSE_REP2")
@@ -86,7 +86,9 @@ public ConsensusCommitManager(
8686
parallelExecutor);
8787
commit = createCommitHandler(config);
8888
isolation = config.getIsolation();
89-
mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager);
89+
operationChecker =
90+
new ConsensusCommitOperationChecker(
91+
tableMetadataManager, config.isIncludeMetadataEnabled());
9092
}
9193

9294
protected ConsensusCommitManager(DatabaseConfig databaseConfig) {
@@ -113,14 +115,17 @@ protected ConsensusCommitManager(DatabaseConfig databaseConfig) {
113115
parallelExecutor);
114116
commit = createCommitHandler(config);
115117
isolation = config.getIsolation();
116-
mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager);
118+
operationChecker =
119+
new ConsensusCommitOperationChecker(
120+
tableMetadataManager, config.isIncludeMetadataEnabled());
117121
}
118122

119123
@SuppressFBWarnings("EI_EXPOSE_REP2")
120124
@VisibleForTesting
121125
ConsensusCommitManager(
122126
DistributedStorage storage,
123127
DistributedStorageAdmin admin,
128+
ConsensusCommitConfig config,
124129
DatabaseConfig databaseConfig,
125130
Coordinator coordinator,
126131
ParallelExecutor parallelExecutor,
@@ -142,8 +147,9 @@ protected ConsensusCommitManager(DatabaseConfig databaseConfig) {
142147
this.commit = commit;
143148
this.groupCommitter = groupCommitter;
144149
this.isolation = isolation;
145-
this.mutationOperationChecker =
146-
new ConsensusCommitMutationOperationChecker(tableMetadataManager);
150+
this.operationChecker =
151+
new ConsensusCommitOperationChecker(
152+
tableMetadataManager, config.isIncludeMetadataEnabled());
147153
}
148154

149155
// `groupCommitter` must be set before calling this method.
@@ -261,7 +267,7 @@ DistributedTransaction begin(
261267
TransactionContext context =
262268
new TransactionContext(txId, snapshot, isolation, readOnly, oneOperation);
263269
DistributedTransaction transaction =
264-
new ConsensusCommit(context, crud, commit, mutationOperationChecker, groupCommitter);
270+
new ConsensusCommit(context, crud, commit, operationChecker, groupCommitter);
265271
if (readOnly) {
266272
transaction = new ReadOnlyDistributedTransaction(transaction);
267273
}

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

Lines changed: 0 additions & 111 deletions
This file was deleted.

0 commit comments

Comments
 (0)