Skip to content

Commit 4811884

Browse files
KodaiDKodaiD
committed
Apply suggestions
1 parent 1231cb7 commit 4811884

13 files changed

+45
-152
lines changed

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.scalar.db.api.ConditionalExpression;
44
import com.scalar.db.api.DistributedStorageConditionalMutationIntegrationTestBase;
5+
import com.scalar.db.io.DataType;
56
import java.util.List;
67
import java.util.Properties;
78
import java.util.stream.Collectors;
@@ -22,9 +23,18 @@ protected int getThreadNum() {
2223
protected List<OperatorAndDataType> getOperatorAndDataTypeListForTest() {
2324
return super.getOperatorAndDataTypeListForTest().stream()
2425
.filter(
25-
operatorAndDataType ->
26-
operatorAndDataType.getOperator() == ConditionalExpression.Operator.EQ
27-
|| operatorAndDataType.getOperator() == ConditionalExpression.Operator.NE)
26+
operatorAndDataType -> {
27+
// Object Storage only supports EQ, NE, IS_NULL, and IS_NOT_NULL conditions for BLOB
28+
// type
29+
if (operatorAndDataType.getDataType() == DataType.BLOB) {
30+
return operatorAndDataType.getOperator() == ConditionalExpression.Operator.EQ
31+
|| operatorAndDataType.getOperator() == ConditionalExpression.Operator.NE
32+
|| operatorAndDataType.getOperator() == ConditionalExpression.Operator.IS_NULL
33+
|| operatorAndDataType.getOperator()
34+
== ConditionalExpression.Operator.IS_NOT_NULL;
35+
}
36+
return true;
37+
})
2838
.collect(Collectors.toList());
2939
}
3040
}

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

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.scalar.db.storage.objectstorage;
22

33
import com.scalar.db.api.DistributedStorageMultipleClusteringKeyScanIntegrationTestBase;
4-
import com.scalar.db.io.Column;
54
import com.scalar.db.io.DataType;
65
import java.util.List;
76
import java.util.Properties;
@@ -27,25 +26,4 @@ protected List<DataType> getDataTypes() {
2726
protected boolean isParallelDdlSupported() {
2827
return false;
2928
}
30-
31-
@Override
32-
protected int getThreadNum() {
33-
return 3;
34-
}
35-
36-
@Override
37-
protected Column<?> getColumnWithMinValue(String columnName, DataType dataType) {
38-
if (dataType == DataType.TEXT) {
39-
return ObjectStorageTestUtils.getMinTextValue(columnName);
40-
}
41-
return super.getColumnWithMinValue(columnName, dataType);
42-
}
43-
44-
@Override
45-
protected Column<?> getColumnWithMaxValue(String columnName, DataType dataType) {
46-
if (dataType == DataType.TEXT) {
47-
return ObjectStorageTestUtils.getMaxTextValue(columnName);
48-
}
49-
return super.getColumnWithMaxValue(columnName, dataType);
50-
}
5129
}

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

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package com.scalar.db.storage.objectstorage;
22

33
import com.scalar.db.api.DistributedStorageMultiplePartitionKeyIntegrationTestBase;
4-
import com.scalar.db.io.Column;
5-
import com.scalar.db.io.DataType;
64
import java.util.Properties;
75

86
public class ObjectStorageMultiplePartitionKeyIntegrationTest
@@ -12,29 +10,8 @@ protected Properties getProperties(String testName) {
1210
return ObjectStorageEnv.getProperties(testName);
1311
}
1412

15-
@Override
16-
protected int getThreadNum() {
17-
return 3;
18-
}
19-
2013
@Override
2114
protected boolean isParallelDdlSupported() {
2215
return false;
2316
}
24-
25-
@Override
26-
protected Column<?> getColumnWithMinValue(String columnName, DataType dataType) {
27-
if (dataType == DataType.TEXT) {
28-
return ObjectStorageTestUtils.getMinTextValue(columnName);
29-
}
30-
return super.getColumnWithMinValue(columnName, dataType);
31-
}
32-
33-
@Override
34-
protected Column<?> getColumnWithMaxValue(String columnName, DataType dataType) {
35-
if (dataType == DataType.TEXT) {
36-
return ObjectStorageTestUtils.getMaxTextValue(columnName);
37-
}
38-
return super.getColumnWithMaxValue(columnName, dataType);
39-
}
4017
}

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.scalar.db.storage.objectstorage;
22

33
import com.scalar.db.api.DistributedStorageSingleClusteringKeyScanIntegrationTestBase;
4-
import com.scalar.db.io.Column;
54
import com.scalar.db.io.DataType;
65
import java.util.ArrayList;
76
import java.util.List;
@@ -26,20 +25,4 @@ protected List<DataType> getClusteringKeyTypes() {
2625
}
2726
return clusteringKeyTypes;
2827
}
29-
30-
@Override
31-
protected Column<?> getColumnWithMinValue(String columnName, DataType dataType) {
32-
if (dataType == DataType.TEXT) {
33-
return ObjectStorageTestUtils.getMinTextValue(columnName);
34-
}
35-
return super.getColumnWithMinValue(columnName, dataType);
36-
}
37-
38-
@Override
39-
protected Column<?> getColumnWithMaxValue(String columnName, DataType dataType) {
40-
if (dataType == DataType.TEXT) {
41-
return ObjectStorageTestUtils.getMaxTextValue(columnName);
42-
}
43-
return super.getColumnWithMaxValue(columnName, dataType);
44-
}
4528
}

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

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package com.scalar.db.storage.objectstorage;
22

33
import com.scalar.db.api.DistributedStorageSinglePartitionKeyIntegrationTestBase;
4-
import com.scalar.db.io.Column;
5-
import com.scalar.db.io.DataType;
64
import java.util.Properties;
75

86
public class ObjectStorageSinglePartitionKeyIntegrationTest
@@ -11,20 +9,4 @@ public class ObjectStorageSinglePartitionKeyIntegrationTest
119
protected Properties getProperties(String testName) {
1210
return ObjectStorageEnv.getProperties(testName);
1311
}
14-
15-
@Override
16-
protected Column<?> getColumnWithMinValue(String columnName, DataType dataType) {
17-
if (dataType == DataType.TEXT) {
18-
return ObjectStorageTestUtils.getMinTextValue(columnName);
19-
}
20-
return super.getColumnWithMinValue(columnName, dataType);
21-
}
22-
23-
@Override
24-
protected Column<?> getColumnWithMaxValue(String columnName, DataType dataType) {
25-
if (dataType == DataType.TEXT) {
26-
return ObjectStorageTestUtils.getMaxTextValue(columnName);
27-
}
28-
return super.getColumnWithMaxValue(columnName, dataType);
29-
}
3012
}

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

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ public enum CoreError implements ScalarDbError {
898898
OBJECT_STORAGE_PRIMARY_KEY_CONTAINS_ILLEGAL_CHARACTER(
899899
Category.USER_ERROR,
900900
"0257",
901-
"The value of the column %s in the primary key contains an illegal character. ",
901+
"The value of the column %s in the primary key contains an illegal character.",
902902
"",
903903
""),
904904
OBJECT_STORAGE_CONDITION_OPERATION_NOT_SUPPORTED_FOR_BLOB_TYPE(

‎core/src/main/java/com/scalar/db/storage/objectstorage/ObjectStorage.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,15 @@ public Optional<Result> get(Get get) throws ExecutionException {
7979
new FilterableScanner(
8080
get, selectStatementHandler.handle(copyAndPrepareForDynamicFiltering(get)));
8181
}
82-
Optional<Result> ret = scanner.one();
83-
if (!scanner.one().isPresent()) {
84-
return ret;
85-
} else {
82+
Optional<Result> result = scanner.one();
83+
if (!result.isPresent()) {
84+
return Optional.empty();
85+
}
86+
if (scanner.one().isPresent()) {
8687
throw new IllegalArgumentException(
8788
CoreError.GET_OPERATION_USED_FOR_NON_EXACT_MATCH_SELECTION.buildMessage(get));
8889
}
90+
return result;
8991
} finally {
9092
if (scanner != null) {
9193
try {

‎core/src/main/java/com/scalar/db/storage/objectstorage/ObjectStorageMutation.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.scalar.db.io.Column;
88
import java.util.Collection;
99
import java.util.Collections;
10+
import java.util.HashMap;
1011
import java.util.Map;
1112
import javax.annotation.Nonnull;
1213
import javax.annotation.concurrent.Immutable;
@@ -22,9 +23,9 @@ public ObjectStorageRecord makeRecord() {
2223
Mutation mutation = (Mutation) getOperation();
2324

2425
if (mutation instanceof Delete) {
25-
return new ObjectStorageRecord();
26+
throw new IllegalStateException("Delete mutation should not make a new record.");
2627
}
27-
Put put = (Put) mutation;
28+
Put put = (Put) getOperation();
2829

2930
return ObjectStorageRecord.newBuilder()
3031
.id(getRecordId())
@@ -40,13 +41,18 @@ public ObjectStorageRecord makeRecord(ObjectStorageRecord existingRecord) {
4041
Mutation mutation = (Mutation) getOperation();
4142

4243
if (mutation instanceof Delete) {
43-
return new ObjectStorageRecord();
44+
throw new IllegalStateException("Delete mutation should not make a new record.");
4445
}
4546
Put put = (Put) mutation;
4647

47-
ObjectStorageRecord newRecord = new ObjectStorageRecord(existingRecord);
48-
toMapForPut(put).forEach((k, v) -> newRecord.getValues().put(k, v));
49-
return newRecord;
48+
Map<String, Object> newValues = new HashMap<>(existingRecord.getValues());
49+
newValues.putAll(toMapForPut(put));
50+
return ObjectStorageRecord.newBuilder()
51+
.id(existingRecord.getId())
52+
.partitionKey(existingRecord.getPartitionKey())
53+
.clusteringKey(existingRecord.getClusteringKey())
54+
.values(newValues)
55+
.build();
5056
}
5157

5258
private Map<String, Object> toMap(Collection<Column<?>> columns) {

‎core/src/main/java/com/scalar/db/storage/objectstorage/ObjectStorageRecord.java‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.scalar.db.storage.objectstorage;
22

3+
import com.fasterxml.jackson.annotation.JsonCreator;
4+
import com.fasterxml.jackson.annotation.JsonProperty;
35
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
46
import java.util.Collections;
57
import java.util.HashMap;
@@ -16,16 +18,12 @@ public class ObjectStorageRecord {
1618
private final Map<String, Object> clusteringKey;
1719
private final Map<String, Object> values;
1820

19-
// The default constructor is required by Jackson to deserialize JSON object
20-
public ObjectStorageRecord() {
21-
this(null, null, null, null);
22-
}
23-
21+
@JsonCreator
2422
public ObjectStorageRecord(
25-
@Nullable String id,
26-
@Nullable Map<String, Object> partitionKey,
27-
@Nullable Map<String, Object> clusteringKey,
28-
@Nullable Map<String, Object> values) {
23+
@JsonProperty("id") @Nullable String id,
24+
@JsonProperty("partitionKey") @Nullable Map<String, Object> partitionKey,
25+
@JsonProperty("clusteringKey") @Nullable Map<String, Object> clusteringKey,
26+
@JsonProperty("values") @Nullable Map<String, Object> values) {
2927
this.id = id != null ? id : "";
3028
this.partitionKey = partitionKey != null ? new HashMap<>(partitionKey) : Collections.emptyMap();
3129
this.clusteringKey =
@@ -42,15 +40,15 @@ public String getId() {
4240
}
4341

4442
public Map<String, Object> getPartitionKey() {
45-
return partitionKey;
43+
return Collections.unmodifiableMap(partitionKey);
4644
}
4745

4846
public Map<String, Object> getClusteringKey() {
49-
return clusteringKey;
47+
return Collections.unmodifiableMap(clusteringKey);
5048
}
5149

5250
public Map<String, Object> getValues() {
53-
return values;
51+
return Collections.unmodifiableMap(values);
5452
}
5553

5654
@Override

0 commit comments

Comments
 (0)