andygrove commented on code in PR #5807:
URL: https://github.com/apache/datafusion-comet/pull/5807#discussion_r3997542844
##########
native/proto/src/proto/operator.proto:
##########
@@ -819,26 +818,25 @@ message RssPartitionWriter {}
message ShuffleWriter {
spark.spark_partitioning.Partitioning partitioning = 1;
// Retained for compatibility with native binaries that predate
partition_writer.
- // Local plans also carry these paths in partition_writer.local.
+ // Local plans also carry this path in partition_writer.local.
string output_data_file = 3;
- string output_index_file = 4;
- CompressionCodec codec = 5;
- int32 compression_level = 6;
- bool tracing_enabled = 7;
+ CompressionCodec codec = 4;
Review Comment:
Please keep the original tag numbers and mark the retired one as `reserved
4; reserved "output_index_file";` instead of shifting `codec` through
`partition_writer` down by one. That is what the rest of this file does for
retired fields, it costs nothing, and it keeps the comment above about
`output_data_file` being retained for older native binaries true. With the
renumbering, an older binary would decode a wire-type mismatch on tag 4.
##########
native/proto/src/proto/operator.proto:
##########
@@ -806,10 +806,9 @@ message PartitionWriter {
}
}
-// Local shuffle output consists of a data file and its partition-offset index.
+// Local shuffle output consists of a data file. The partition offsets are
returned to the JVM via JNI
Review Comment:
Same for `LocalPartitionWriter`. Tag 2 is now silently free, so a `reserved
2; reserved "output_index_file";` here would keep someone from reusing it for a
different field later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
