Skip to content

Clean up field presence checks and tests in anticipation of Protobuf Editions#3920

Merged
alecgrieser merged 3 commits intoFoundationDB:mainfrom
MMcM:proto3-explicit-presence
Feb 5, 2026
Merged

Clean up field presence checks and tests in anticipation of Protobuf Editions#3920
alecgrieser merged 3 commits intoFoundationDB:mainfrom
MMcM:proto3-explicit-presence

Conversation

@MMcM
Copy link
Collaborator

@MMcM MMcM commented Feb 5, 2026

MMcM added 2 commits February 4, 2026 15:32
…tional.

Also, changing this in either direction is problematic as to whether default values are indexed as nulls.
@MMcM MMcM marked this pull request as ready for review February 5, 2026 00:46
@MMcM MMcM added the enhancement New feature or request label Feb 5, 2026
final String labelText = label.name().substring(label.name().indexOf('_') + 1).toLowerCase(Locale.ROOT);
errMsg = labelText + " field is no longer " + labelText;
}
final DescriptorProtos.FieldDescriptorProto.Label newLabel = labels.get((i + 1) % labels.size());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, we probably want a test like "compare a proto2/proto3 file with explicit optionals to a file with editions set correctly/incorrectly", but I think we throw an error right now if the syntax doesn't match between two files anyway, so that test is probably pre-mature. This change, on the whole, seems to put us closer to that future, though

@alecgrieser alecgrieser merged commit 0b611fb into FoundationDB:main Feb 5, 2026
10 of 11 checks passed
@MMcM MMcM deleted the proto3-explicit-presence branch February 5, 2026 18:23
g31pranjal pushed a commit to g31pranjal/fdb-record-layer that referenced this pull request Feb 6, 2026
…Editions (FoundationDB#3920)

* Add a test of Proto3 with explicit presence.
* Use `hasPresence` instead of `isOptional` in evolution validator.
* See FoundationDB#3919 for confirmation that compiling with a newer Proto
dependency will work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants