Protobuf generate V2 and V3 protos and extend tests.

[rangadi]

This is a temporary PR on, made on top of Java-support PR #37738.

Outline:

• Generates descriptor files and java classes for both V2 and V3
• Updates the unit tests to check all variations (V2 & V3 x Java, decriptor file).
• Adds a test with default values and required fields for V2.
• Maven build is updated. But not clear how do the same in SBT.

[rangadi]

FYI: @mposdev21, @SandishKumarHN

[mposdev21]

Overall, looks good to me except for a few things I noted.

[mposdev21]

I am not sure I understand the comment completely. When buildDescriptor is called, the contents of the file is read and a descriptor is created. The first time messageDescriptor is evaluated, the file contents are read. Could you clarify ?

[rangadi]

I had added this in my Java class pr #38286.
The file contents are read on executors too when the ProtobufDataToCatalyst() class is initialized there. Notice @transient private lazy val messageDescriptor in the class.

[mposdev21]

Do we still want to keep these comments ?

[rangadi]

Removed. Also added pyspark_test.proto to this directory.

[mposdev21]

Can we also add Enum ? I tested with a default value which seemed to work. But then the following text in the spec:

"For enums, the default value is the first value listed in the enum's type definition."

If I interpret this to mean that if we don't specify a default value and the field is left uninitialized, it should pick the. first default value. That interpretation does not work because hasDefaultValue fails and hence the test would fail if you assert that it has the first value.

[mposdev21]

Should we fix this in v2 and v3 ? Seems like a small change.

[mposdev21]

It looks like we still have the desc files under resources/protobuf ?

[rangadi]

Fixed now.

[mposdev21]

Here SimpleMessageRepeated is a v3 class which is used to build the protobuf message. It might make sense to add one test case to use v2.SimpleMessageRepeated (or anything else) to build the message and do the test.

[mposdev21]

I guess. you are picking the descriptors (v2 or v3) for building the message randomly here. Is that correct ?

[rangadi]

Yes.

[LuciferYang]

A newbie question: why we need support protobuf v2?

[AmplabJenkins]

Can one of the admins verify this patch?

[rangadi]

A newbie question: why we need support protobuf v2?

@LuciferYang many users still use v2 protobufs. It is not always easy to migrate the protobufs to v3 or even necessary. Note that v2 protobufs are still genrated with protoc 3.x.x. The v2 protos used in tests are built with 3.2.1. The syntax version (v2 or v3) is different from version used to parse and generate Java classes or descriptor sets.

[github-actions]

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!