Skip to content

Commit c65d1c3

Browse files
committed
Fix based on feedback
1 parent 160922f commit c65d1c3

File tree

2 files changed

+74
-1
lines changed

2 files changed

+74
-1
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ public void check(Scan scan, TransactionContext context) throws ExecutionExcepti
159159
for (Selection.Conjunction conjunction : scan.getConjunctions()) {
160160
for (ConditionalExpression condition : conjunction.getConditions()) {
161161
String column = condition.getColumn().getName();
162-
if (metadata.getSecondaryIndexNames().contains(column)) {
162+
// If the column is an indexed column but is part of the primary key, it's allowed
163+
if (metadata.getSecondaryIndexNames().contains(column)
164+
&& !tableMetadata.getPartitionKeyNames().contains(column)
165+
&& !tableMetadata.getClusteringKeyNames().contains(column)) {
163166
throw new IllegalArgumentException(
164167
CoreError
165168
.CONSENSUS_COMMIT_CONDITION_ON_INDEXED_COLUMNS_NOT_ALLOWED_IN_CROSS_PARTITION_SCAN_IN_SERIALIZABLE

core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitOperationCheckerTest.java

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,12 @@ public void checkForScan_WithIndexKeyUsingClusteringKeyInSerializable_ShouldNotT
495495
Set<String> secondaryIndexNames = new LinkedHashSet<>(Collections.singletonList("idx_col"));
496496
when(tableMetadata.getSecondaryIndexNames()).thenReturn(secondaryIndexNames);
497497

498+
// Mock the underlying TableMetadata
499+
TableMetadata mockTableMetadata = mock(TableMetadata.class);
500+
when(tableMetadata.getTableMetadata()).thenReturn(mockTableMetadata);
501+
when(mockTableMetadata.getPartitionKeyNames()).thenReturn(new LinkedHashSet<>());
502+
when(mockTableMetadata.getClusteringKeyNames()).thenReturn(new LinkedHashSet<>());
503+
498504
Scan scan =
499505
ScanAll.newBuilder()
500506
.namespace("ns")
@@ -517,6 +523,12 @@ public void checkForScan_WithIndexKeyUsingClusteringKeyInSerializable_ShouldNotT
517523
Set<String> secondaryIndexNames = new LinkedHashSet<>(Collections.singletonList("idx_col"));
518524
when(tableMetadata.getSecondaryIndexNames()).thenReturn(secondaryIndexNames);
519525

526+
// Mock the underlying TableMetadata
527+
TableMetadata mockTableMetadata = mock(TableMetadata.class);
528+
when(tableMetadata.getTableMetadata()).thenReturn(mockTableMetadata);
529+
when(mockTableMetadata.getPartitionKeyNames()).thenReturn(new LinkedHashSet<>());
530+
when(mockTableMetadata.getClusteringKeyNames()).thenReturn(new LinkedHashSet<>());
531+
520532
Scan scan =
521533
ScanAll.newBuilder()
522534
.namespace("ns")
@@ -531,6 +543,64 @@ public void checkForScan_WithIndexKeyUsingClusteringKeyInSerializable_ShouldNotT
531543
assertThatCode(() -> checker.check(scan, context)).doesNotThrowAnyException();
532544
}
533545

546+
@Test
547+
public void
548+
checkForScan_ScanAllWithConditionOnIndexedPartitionKeyColumnInSerializable_ShouldNotThrowException() {
549+
// Arrange
550+
Set<String> secondaryIndexNames = new LinkedHashSet<>(Collections.singletonList("idx_col"));
551+
when(tableMetadata.getSecondaryIndexNames()).thenReturn(secondaryIndexNames);
552+
553+
// Mock the underlying TableMetadata - indexed column is also a partition key
554+
TableMetadata mockTableMetadata = mock(TableMetadata.class);
555+
when(tableMetadata.getTableMetadata()).thenReturn(mockTableMetadata);
556+
LinkedHashSet<String> partitionKeys = new LinkedHashSet<>();
557+
partitionKeys.add("idx_col");
558+
when(mockTableMetadata.getPartitionKeyNames()).thenReturn(partitionKeys);
559+
when(mockTableMetadata.getClusteringKeyNames()).thenReturn(new LinkedHashSet<>());
560+
561+
Scan scan =
562+
ScanAll.newBuilder()
563+
.namespace("ns")
564+
.table("tbl")
565+
.all()
566+
.where(ConditionBuilder.column("idx_col").isEqualToInt(100))
567+
.build();
568+
TransactionContext context =
569+
new TransactionContext("txId", null, Isolation.SERIALIZABLE, false, false);
570+
571+
// Act Assert
572+
assertThatCode(() -> checker.check(scan, context)).doesNotThrowAnyException();
573+
}
574+
575+
@Test
576+
public void
577+
checkForScan_ScanAllWithConditionOnIndexedClusteringKeyColumnInSerializable_ShouldNotThrowException() {
578+
// Arrange
579+
Set<String> secondaryIndexNames = new LinkedHashSet<>(Collections.singletonList("idx_col"));
580+
when(tableMetadata.getSecondaryIndexNames()).thenReturn(secondaryIndexNames);
581+
582+
// Mock the underlying TableMetadata - indexed column is also a clustering key
583+
TableMetadata mockTableMetadata = mock(TableMetadata.class);
584+
when(tableMetadata.getTableMetadata()).thenReturn(mockTableMetadata);
585+
when(mockTableMetadata.getPartitionKeyNames()).thenReturn(new LinkedHashSet<>());
586+
LinkedHashSet<String> clusteringKeys = new LinkedHashSet<>();
587+
clusteringKeys.add("idx_col");
588+
when(mockTableMetadata.getClusteringKeyNames()).thenReturn(clusteringKeys);
589+
590+
Scan scan =
591+
ScanAll.newBuilder()
592+
.namespace("ns")
593+
.table("tbl")
594+
.all()
595+
.where(ConditionBuilder.column("idx_col").isEqualToInt(100))
596+
.build();
597+
TransactionContext context =
598+
new TransactionContext("txId", null, Isolation.SERIALIZABLE, false, false);
599+
600+
// Act Assert
601+
assertThatCode(() -> checker.check(scan, context)).doesNotThrowAnyException();
602+
}
603+
534604
@Test
535605
public void
536606
checkForScan_RegularScanWithConditionOnIndexedColumnInSerializable_ShouldNotThrowException() {

0 commit comments

Comments
 (0)