Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot compile protos to JAVA #179

Open
bmvbooris opened this issue Feb 20, 2023 · 3 comments
Open

Cannot compile protos to JAVA #179

bmvbooris opened this issue Feb 20, 2023 · 3 comments

Comments

@bmvbooris
Copy link

When I try to compile the protos in java I get the following error (i'm using maven to build it) :

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project pg-query: Compilation failure: Compilation failure: 
[ERROR] /home/tpl/prism/target/generated-sources/protobuf/java/com/tora/prism/pg/query/PgQuery.java:[14216,34] variable VALUES is already defined in enum com.tora.prism.pg.query.PgQuery.Token
[ERROR] /home/tpl/prism/target/generated-sources/protobuf/java/com/tora/prism/pg/query/PgQuery.java:[14227,20] array required, but com.tora.prism.pg.query.PgQuery.Token found
[ERROR] -> [Help 1]
   org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project pg-query: Compilation failure

The issue seems to be the enum field in VALUES in the Token enum. Apparently, when you compile to java it already inserts a static field named VALUES for all enums. There is a workaround, simply rename the enum to VALUE and it works. Knowing how protos are serialized, this should not break the serialization/deserialization of the protos. However we need to fix this.

Strictly for java it would also be nice to add the following options to the protos (either way):

option java_multiple_files = false;
option java_package = "a nice package name";
option java_outer_classname = "A nice class name";

or

option java_multiple_files = true;
option java_package = "a nice package name";
@lfittl
Copy link
Member

lfittl commented Feb 22, 2023

@bmvbooris Thanks for raising this!

That's an interesting issue, because VALUES is the name the Postgres source uses for that scan token - we're just using that name 1:1 (and the enum its used in is auto-generated).

Have you found any documentation upstream that explicitly mentions VALUES not being a permitted message field name? I'm surprised they don't just auto-rename the field in case of such conflicts.

That said, from a what to do about it perspective: Do you know if there is a way to override the field name just for Java, instead of changing this for everyone?

For context, changing enum names is technically an API break in our use of protobufs, since consumers might have generated language-specific enums based on our protobuf definition, and so if they upgrade they'd suddenly get different auto-generated enum field names, breaking their application. If we can, it would be great if we can avoid having to change this for everyone.

Also, out of curiosity, for your use case, are you looking at the scanner (where the VALUES name is used), or the parser? (the parser is really the main part of pg_query, which only uses "values" in one of the messages, but seems like that might be okay) -- one way to solve this in the short-term might be to just split up the protobuf definition files for the parser and the scanner, that way you can only generate the one you're interested in (i.e. hopefully the parser 😄)

@mpokryva
Copy link

mpokryva commented Mar 9, 2023

Ran into the same issue. This Sonar plugin solves the problem via a patch file. The same approach has worked well for us.

@mpokryva
Copy link

mpokryva commented Mar 9, 2023

The GitHub issue, btw, is protocolbuffers/protobuf#9264

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

No branches or pull requests

3 participants