Skip to content

Commit 799c3fb

Browse files
brfrn169brfrn169
committed
Merge branch 'master' into support-transaction-metadata-decoupling-in-consensus-commit
2 parents f812da1 + d4a3a97 commit 799c3fb

File tree

7 files changed

+699
-11
lines changed

7 files changed

+699
-11
lines changed

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,15 +1009,21 @@ public enum CoreError implements ScalarDbError {
10091009
"Source tables cannot be dropped while virtual tables depending on them exist. Source table: %s; Virtual tables: %s",
10101010
"",
10111011
""),
1012-
CONSENSUS_COMMIT_TRANSACTION_METADATA_DECOUPLING_NOT_SUPPORTED_STORAGE(
1012+
DELETE_IF_IS_NULL_FOR_RIGHT_SOURCE_TABLE_NOT_ALLOWED_FOR_LEFT_OUTER_VIRTUAL_TABLES(
10131013
Category.USER_ERROR,
10141014
"0276",
1015+
"The DeleteIf IS_NULL condition for right source table columns is not allowed in LEFT_OUTER virtual tables. Virtual table: %s",
1016+
"",
1017+
""),
1018+
CONSENSUS_COMMIT_TRANSACTION_METADATA_DECOUPLING_NOT_SUPPORTED_STORAGE(
1019+
Category.USER_ERROR,
1020+
"0277",
10151021
"The transaction metadata decoupling feature is not supported in the storage. Storage %s",
10161022
"",
10171023
""),
10181024
CONSENSUS_COMMIT_TRANSACTION_METADATA_CONSISTENT_READS_NOT_GUARANTEED_STORAGE(
10191025
Category.USER_ERROR,
1020-
"0277",
1026+
"0278",
10211027
"The storage does not guarantee consistent reads for virtual tables. Depending on the storage configuration, "
10221028
+ "you may be able to adjust the settings to enable consistent reads. Please refer to the storage configuration for details. Storage: %s",
10231029
"",

‎core/src/main/java/com/scalar/db/common/VirtualTableInfoManager.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,25 @@
99
import com.scalar.db.exception.storage.ExecutionException;
1010
import com.scalar.db.util.ScalarDbUtils;
1111
import java.util.Objects;
12+
import java.util.Optional;
1213
import java.util.concurrent.CompletionException;
1314
import java.util.concurrent.TimeUnit;
1415
import javax.annotation.Nullable;
1516
import javax.annotation.concurrent.ThreadSafe;
1617

17-
/** A class that manages and caches virtual table information */
18+
/** A class that manages and caches virtual table information. */
1819
@ThreadSafe
1920
public class VirtualTableInfoManager {
2021

21-
private final LoadingCache<TableKey, VirtualTableInfo> virtualTableInfoCache;
22+
private final LoadingCache<TableKey, Optional<VirtualTableInfo>> virtualTableInfoCache;
2223

2324
public VirtualTableInfoManager(DistributedStorageAdmin admin, long cacheExpirationTimeSecs) {
2425
Caffeine<Object, Object> builder = Caffeine.newBuilder();
2526
if (cacheExpirationTimeSecs >= 0) {
2627
builder.expireAfterWrite(cacheExpirationTimeSecs, TimeUnit.SECONDS);
2728
}
2829
virtualTableInfoCache =
29-
builder.build(key -> admin.getVirtualTableInfo(key.namespace, key.table).orElse(null));
30+
builder.build(key -> admin.getVirtualTableInfo(key.namespace, key.table));
3031
}
3132

3233
/**
@@ -60,7 +61,9 @@ public VirtualTableInfo getVirtualTableInfo(String namespace, String table)
6061
throws ExecutionException {
6162
try {
6263
TableKey key = new TableKey(namespace, table);
63-
return virtualTableInfoCache.get(key);
64+
Optional<VirtualTableInfo> virtualTableInfo = virtualTableInfoCache.get(key);
65+
assert virtualTableInfo != null;
66+
return virtualTableInfo.orElse(null);
6467
} catch (CompletionException e) {
6568
throw new ExecutionException(
6669
CoreError.GETTING_VIRTUAL_TABLE_INFO_FAILED.buildMessage(

‎core/src/main/java/com/scalar/db/storage/jdbc/JdbcDatabase.java‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.scalar.db.exception.storage.NoMutationException;
3636
import com.scalar.db.exception.storage.RetriableExecutionException;
3737
import com.scalar.db.io.Column;
38+
import com.scalar.db.util.ScalarDbUtils;
3839
import java.sql.Connection;
3940
import java.sql.SQLException;
4041
import java.util.ArrayList;
@@ -395,7 +396,19 @@ private List<Put> dividePutForSourceTables(Put put, VirtualTableInfo virtualTabl
395396
putBuilderForLeftSourceTable.condition(ConditionBuilder.putIf(leftExpressions));
396397
}
397398
if (!rightExpressions.isEmpty()) {
398-
putBuilderForRightSourceTable.condition(ConditionBuilder.putIf(rightExpressions));
399+
if (isAllIsNullOnRightColumnsInLeftOuterJoin(virtualTableInfo, rightExpressions)
400+
&& JdbcOperationAttributes
401+
.isLeftOuterVirtualTablePutIfIsNullOnRightColumnsConversionEnabled(put)) {
402+
// In a LEFT_OUTER join, when all conditions on the right source table columns are
403+
// IS_NULL, we cannot distinguish whether we should check for the existence of a
404+
// right-side record with NULL values or for the case where the right-side record does
405+
// not exist at all. Therefore, this behavior is controlled by the operation attribute.
406+
// By default, we convert the condition to PutIfNotExists, assuming that the more common
407+
// use case is to check that the right-side record does not exist.
408+
putBuilderForRightSourceTable.condition(ConditionBuilder.putIfNotExists());
409+
} else {
410+
putBuilderForRightSourceTable.condition(ConditionBuilder.putIf(rightExpressions));
411+
}
399412
}
400413
}
401414
}
@@ -464,7 +477,25 @@ private List<Delete> divideDeleteForSourceTables(Delete delete, VirtualTableInfo
464477
deleteBuilderForLeftSourceTable.condition(ConditionBuilder.deleteIf(leftExpressions));
465478
}
466479
if (!rightExpressions.isEmpty()) {
467-
deleteBuilderForRightSourceTable.condition(ConditionBuilder.deleteIf(rightExpressions));
480+
if (isAllIsNullOnRightColumnsInLeftOuterJoin(virtualTableInfo, rightExpressions)
481+
&& !JdbcOperationAttributes
482+
.isLeftOuterVirtualTableDeleteIfIsNullOnRightColumnsAllowed(delete)) {
483+
// In a LEFT_OUTER join, when all conditions on the right source table columns are
484+
// IS_NULL, we cannot distinguish whether we should check for the existence of a
485+
// right-side record with NULL values or for the case where the right-side record does
486+
// not exist at all. This makes the delete operation semantically ambiguous. Therefore,
487+
// this behavior is controlled by the operation attribute. By default, we disallow this
488+
// operation to prevent unintended behavior.
489+
assert delete.forNamespace().isPresent() && delete.forTable().isPresent();
490+
throw new IllegalArgumentException(
491+
CoreError
492+
.DELETE_IF_IS_NULL_FOR_RIGHT_SOURCE_TABLE_NOT_ALLOWED_FOR_LEFT_OUTER_VIRTUAL_TABLES
493+
.buildMessage(
494+
ScalarDbUtils.getFullTableName(
495+
delete.forNamespace().get(), delete.forTable().get())));
496+
} else {
497+
deleteBuilderForRightSourceTable.condition(ConditionBuilder.deleteIf(rightExpressions));
498+
}
468499
}
469500
}
470501
}
@@ -474,6 +505,13 @@ private List<Delete> divideDeleteForSourceTables(Delete delete, VirtualTableInfo
474505
return Arrays.asList(deleteForLeftSourceTable, deleteForRightSourceTable);
475506
}
476507

508+
private boolean isAllIsNullOnRightColumnsInLeftOuterJoin(
509+
VirtualTableInfo virtualTableInfo, List<ConditionalExpression> rightExpressions) {
510+
return virtualTableInfo.getJoinType() == VirtualTableJoinType.LEFT_OUTER
511+
&& rightExpressions.stream()
512+
.allMatch(e -> e.getOperator() == ConditionalExpression.Operator.IS_NULL);
513+
}
514+
477515
private void close(Connection connection) {
478516
try {
479517
if (connection != null) {

‎core/src/main/java/com/scalar/db/storage/jdbc/JdbcOperationAttributes.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package com.scalar.db.storage.jdbc;
2+
3+
import com.scalar.db.api.Delete;
4+
import com.scalar.db.api.Put;
5+
import java.util.Map;
6+
7+
/** A utility class to manipulate the operation attributes for JDBC. */
8+
public final class JdbcOperationAttributes {
9+
private static final String OPERATION_ATTRIBUTE_PREFIX = "jdbc-";
10+
11+
public static final String
12+
LEFT_OUTER_VIRTUAL_TABLE_PUT_IF_IS_NULL_ON_RIGHT_COLUMNS_CONVERSION_ENABLED =
13+
OPERATION_ATTRIBUTE_PREFIX
14+
+ "left-outer-virtual-table-put-if-is-null-on-right-columns-conversion-enabled";
15+
16+
public static final String LEFT_OUTER_VIRTUAL_TABLE_DELETE_IF_IS_NULL_ON_RIGHT_COLUMNS_ALLOWED =
17+
OPERATION_ATTRIBUTE_PREFIX
18+
+ "left-outer-virtual-table-delete-if-is-null-on-right-columns-allowed";
19+
20+
private JdbcOperationAttributes() {}
21+
22+
public static boolean isLeftOuterVirtualTablePutIfIsNullOnRightColumnsConversionEnabled(Put put) {
23+
return put.getAttribute(
24+
LEFT_OUTER_VIRTUAL_TABLE_PUT_IF_IS_NULL_ON_RIGHT_COLUMNS_CONVERSION_ENABLED)
25+
.map(Boolean::parseBoolean)
26+
.orElse(true);
27+
}
28+
29+
public static void setLeftOuterVirtualTablePutIfIsNullOnRightColumnsConversionEnabled(
30+
Map<String, String> attributes, boolean enabled) {
31+
attributes.put(
32+
LEFT_OUTER_VIRTUAL_TABLE_PUT_IF_IS_NULL_ON_RIGHT_COLUMNS_CONVERSION_ENABLED,
33+
Boolean.toString(enabled));
34+
}
35+
36+
public static boolean isLeftOuterVirtualTableDeleteIfIsNullOnRightColumnsAllowed(Delete delete) {
37+
return delete
38+
.getAttribute(LEFT_OUTER_VIRTUAL_TABLE_DELETE_IF_IS_NULL_ON_RIGHT_COLUMNS_ALLOWED)
39+
.map(Boolean::parseBoolean)
40+
.orElse(false);
41+
}
42+
43+
public static void setLeftOuterVirtualTableDeleteIfIsNullOnRightColumnsAllowed(
44+
Map<String, String> attributes, boolean allowed) {
45+
attributes.put(
46+
LEFT_OUTER_VIRTUAL_TABLE_DELETE_IF_IS_NULL_ON_RIGHT_COLUMNS_ALLOWED,
47+
Boolean.toString(allowed));
48+
}
49+
}

‎core/src/test/java/com/scalar/db/common/VirtualTableInfoManagerTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ public void getVirtualTableInfo_WithCacheExpiration_ShouldExpireCache() throws E
157157
}
158158

159159
@Test
160-
public void getVirtualTableInfo_VirtualTableDoesNotExist_ShouldNotCacheNullResult()
160+
public void getVirtualTableInfo_VirtualTableDoesNotExist_ShouldCacheEmptyResult()
161161
throws Exception {
162162
// Arrange
163163
manager = new VirtualTableInfoManager(admin, -1);
@@ -168,8 +168,8 @@ public void getVirtualTableInfo_VirtualTableDoesNotExist_ShouldNotCacheNullResul
168168
manager.getVirtualTableInfo("ns", "table");
169169
manager.getVirtualTableInfo("ns", "table");
170170

171-
// Assert - admin should be called multiple times because null is not cached
172-
verify(admin, times(3)).getVirtualTableInfo("ns", "table");
171+
// Assert - admin should be called only once because Optional.empty() is cached
172+
verify(admin, times(1)).getVirtualTableInfo("ns", "table");
173173
}
174174

175175
@Test

0 commit comments

Comments
 (0)