Describe the bug
Running regen.sh in datafusion-proto fails with the error:
$ ./regen.sh
Error: "protobuf compilation failed: protoc failed: datafusion/proto/proto/datafusion.proto: This file contains proto3 optional fields, but --experimental_allow_proto3_optional was not set.\n"
The error happens with libprotoc 3.12.4, which is the version recommended in the DataFusion docs. This is also the default version installed by apt in Ubuntu 22.04.
$ protoc --version
libprotoc 3.12.4
A similar bug was reported in #8602.
I needed to run regen.sh while trying to migrate string_agg, one of the built-in aggregate functions pending migration, listed in #8708.
Could you recommend an approach to fix this? I have listed 3 possible ways here.
1. Add flag
Satisfy the experimental check by passing --experimental_allow_proto3_optional to the protoc compiler invocation. I've verified locally that this will work.
// In datafusion/proto/gen/src/main.rs L31
prost_build::Config::new()
.protoc_arg("--experimental_allow_proto3_optional")
2. Convert to oneof
Rewrite optional field as a oneof with a single field.
Currently, in datafusion.proto there are two usages of optional:
message ScalarUDFExprNode {
string fun_name = 1;
repeated LogicalExprNode args = 2;
optional bytes fun_definition = 3;
}
message PhysicalScalarUdfNode {
string name = 1;
repeated PhysicalExprNode args = 2;
optional bytes fun_definition = 3;
datafusion_common.ArrowType return_type = 4;
}
We can rewrite the optional field as a oneof with a single field like this:
message ScalarUDFExprNode {
string fun_name = 1;
repeated LogicalExprNode args = 2;
oneof fun_definition_opt {
bytes fun_definition = 3;
}
}
This technique is already used in other message types in datafusion.proto like - WindowFrame, CsvScanExecNode, ProjectionNode etc.
I've tried this change in my fork and verified that it works. Please see the diff here
3. Remove optional
We could remove the optional keyword usage here, which would fix the issue. It might however have other cascading effects. To evaluate it, it might be useful to see #8706, where the message type in question was designed.
To Reproduce
You need protoc version 3.12.4.
Expected behavior
Running regen.sh should exit without errors when using protoc version 3.12.4.
Additional context
No response
Describe the bug
Running
regen.shindatafusion-protofails with the error:$ ./regen.sh Error: "protobuf compilation failed: protoc failed: datafusion/proto/proto/datafusion.proto: This file contains proto3 optional fields, but --experimental_allow_proto3_optional was not set.\n"The error happens with libprotoc 3.12.4, which is the version recommended in the DataFusion docs. This is also the default version installed by apt in Ubuntu 22.04.
A similar bug was reported in #8602.
I needed to run
regen.shwhile trying to migratestring_agg, one of the built-in aggregate functions pending migration, listed in #8708.Could you recommend an approach to fix this? I have listed 3 possible ways here.
1. Add flag
Satisfy the experimental check by passing
--experimental_allow_proto3_optionalto theprotoccompiler invocation. I've verified locally that this will work.2. Convert to oneof
Rewrite
optionalfield as aoneofwith a single field.Currently, in
datafusion.protothere are two usages ofoptional:We can rewrite the
optionalfield as aoneofwith a single field like this:This technique is already used in other message types in
datafusion.protolike -WindowFrame,CsvScanExecNode,ProjectionNodeetc.I've tried this change in my fork and verified that it works. Please see the diff here
3. Remove optional
We could remove the
optionalkeyword usage here, which would fix the issue. It might however have other cascading effects. To evaluate it, it might be useful to see #8706, where the message type in question was designed.To Reproduce
You need
protocversion3.12.4.Expected behavior
Running
regen.shshould exit without errors when usingprotocversion3.12.4.Additional context
No response