Skip to content

Conversation

@uwemaurer
Copy link

Rationale for this change

The getSyntax method on messageDescriptor.getFile() got removed in protobuf version 4.
It crashes when a user updates to protobuf 4 and uses the parquet library.

What changes are included in this PR?

This change gets the syntax information directly from the proto, this works in protobuf v3 and v4.

Are these changes tested?

yes, running the existing tests (with protobuf 3).

Are there any user-facing changes?

Closes #3175

Related to #3182

The getSyntax method on messageDescriptor.getFile() got removed
in protobuf version 4.
This change gets the syntax information directly from the proto, this
works in protobuf v3 and v4.


fixes apache#3175

see also apache#3182
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

This makes sense to me 👍

if ("editions".equals(syntax)) {
throw new UnsupportedOperationException("protocol buffers 'editions' not supported");
}
boolean isProto2 = !"proto3".equals(syntax);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to check the versions explicitly, and raise on anything else (for example editions, as above).

Suggested change
boolean isProto2 = !"proto3".equals(syntax);
boolean isProto2 = "proto2".equals(syntax);

Copy link
Author

@uwemaurer uwemaurer Oct 26, 2025

Choose a reason for hiding this comment

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

I wanted to keep the logic the same as in the protobuf library (v4), they have this code:

 Edition getEdition() {
      switch (proto.getSyntax()) {
        case "editions":
          return proto.getEdition();
        case "proto3":
          return Edition.EDITION_PROTO3;
        default:
          return Edition.EDITION_PROTO2;
      }
    }

In my tests the value of syntax was an empty string (not "proto2") for proto2 files, this is why it just compares against "editions" and "proto3".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for protobuf version 4.29.x

3 participants