Skip to content

Commit 22e28f8

Browse files
committed
Update meta-data evolution validator to check presence rather than optional.
Also, changing this in either direction is problematic as to whether default values are indexed as nulls.
1 parent e67b1b3 commit 22e28f8

3 files changed

Lines changed: 50 additions & 9 deletions

File tree

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/metadata/MetaDataEvolutionValidator.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,12 @@ private void validateField(@Nonnull FieldDescriptor oldFieldDescriptor, @Nonnull
274274
if (oldFieldDescriptor.isRequired() && !newFieldDescriptor.isRequired()) {
275275
throw new MetaDataException("required field is no longer required",
276276
LogMessageKeys.FIELD_NAME, oldFieldDescriptor.getName());
277-
} else if (oldFieldDescriptor.isOptional() && !newFieldDescriptor.isOptional()) {
278-
// TODO: In theory, optional -> repeated is okay, but only if the field is not indexed
279-
throw new MetaDataException("optional field is no longer optional",
280-
LogMessageKeys.FIELD_NAME, oldFieldDescriptor.getName());
281277
} else if (oldFieldDescriptor.isRepeated() && !newFieldDescriptor.isRepeated()) {
282278
throw new MetaDataException("repeated field is no longer repeated",
283279
LogMessageKeys.FIELD_NAME, oldFieldDescriptor.getName());
280+
} else if (oldFieldDescriptor.hasPresence() != newFieldDescriptor.hasPresence()) {
281+
throw new MetaDataException("field changed whether default values are stored if set explicitly",
282+
LogMessageKeys.FIELD_NAME, oldFieldDescriptor.getName());
284283
}
285284
if (oldFieldDescriptor.getType().equals(FieldDescriptor.Type.ENUM)) {
286285
validateEnum(newFieldDescriptor.getName(), oldFieldDescriptor.getEnumType(), newFieldDescriptor.getEnumType());

fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/metadata/MetaDataEvolutionValidatorTest.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -857,12 +857,17 @@ public void fieldLabelChanged() {
857857
DescriptorProtos.FieldDescriptorProto.Label.LABEL_REQUIRED
858858
);
859859
for (int i = 0; i < labels.size(); i++) {
860-
final int itr = i;
861-
final DescriptorProtos.FieldDescriptorProto.Label label = labels.get(itr);
862-
final String labelText = label.name().substring(label.name().indexOf('_') + 1).toLowerCase(Locale.ROOT);
863-
final String errMsg = labelText + " field is no longer " + labelText;
860+
final DescriptorProtos.FieldDescriptorProto.Label label = labels.get(i);
861+
final String errMsg;
862+
if (label == DescriptorProtos.FieldDescriptorProto.Label.LABEL_OPTIONAL) {
863+
errMsg = "field changed whether default values are stored if set explicitly";
864+
} else {
865+
final String labelText = label.name().substring(label.name().indexOf('_') + 1).toLowerCase(Locale.ROOT);
866+
errMsg = labelText + " field is no longer " + labelText;
867+
}
868+
final DescriptorProtos.FieldDescriptorProto.Label newLabel = labels.get((i + 1) % labels.size());
864869
FileDescriptor updatedFile = mutateField("MySimpleRecord", "str_value_indexed", oldFile,
865-
field -> field.setLabel(labels.get((itr + 1) % labels.size())));
870+
field -> field.setLabel(newLabel));
866871
assertInvalid(errMsg, oldFile, updatedFile);
867872

868873
oldFile = updatedFile;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* test_records_nulls_3_explicit.proto
3+
*
4+
* This source file is part of the FoundationDB open source project
5+
*
6+
* Copyright 2026 Apple Inc. and the FoundationDB project authors
7+
*
8+
* Licensed under the Apache License, Version 2.0 (the "License");
9+
* you may not use this file except in compliance with the License.
10+
* You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing, software
15+
* distributed under the License is distributed on an "AS IS" BASIS,
16+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
*/
20+
syntax = "proto3";
21+
22+
package com.apple.foundationdb.record.testnulls3explicit;
23+
24+
option java_package = "com.apple.foundationdb.record";
25+
option java_outer_classname = "TestRecordsNulls3ExplicitProto";
26+
27+
import "record_metadata_options.proto";
28+
29+
message MyNullRecord {
30+
optional string name = 1 [(field).primary_key = true];
31+
optional string string_value = 2 [(field).index = {}];
32+
optional int32 int_value = 3 [(field).index = {}];
33+
}
34+
35+
message RecordTypeUnion {
36+
optional MyNullRecord _MyNullRecord = 1;
37+
}

0 commit comments

Comments
 (0)