Copilot commented on code in PR #12841:
URL: https://github.com/apache/gluten/pull/12841#discussion_r3824657043
##########
gluten-substrait/src/main/resources/substrait/proto/substrait/algebra.proto:
##########
@@ -69,7 +69,7 @@ message ReadRel {
LocalFiles local_files = 6;
NamedTable named_table = 7;
Review Comment:
The PR description says the deleted `stream_kafka` graft tag (1000) is
marked `reserved` to prevent reuse, but `ReadRel` doesn’t reserve it. Without a
`reserved` declaration, a future edit could accidentally reuse 1000 /
`stream_kafka` and silently break compatibility.
##########
gluten-substrait/src/main/scala/org/apache/gluten/utils/SubstraitPlanPrinterUtil.scala:
##########
@@ -31,6 +33,9 @@ object SubstraitPlanPrinterUtil extends Logging {
.newBuilder()
.add(d)
.add(defaultRegistry)
+ // Gluten's own payloads (e.g. StreamKafka) ride in Any fields of the
Substrait plan, and
+ // nothing imports kafka.proto, so its messages are not reachable from
the plan descriptor.
+ .add(Kafka.getDescriptor.getMessageTypes)
Review Comment:
The new import `org.apache.gluten.proto.Kafka` is likely not a generated
class for `kafka.proto` (there is no `message Kafka`, and with
`java_multiple_files = true` the generated outer class name is typically
`KafkaProto`). This will break compilation and also isn’t necessary for
TypeRegistry registration; you can register the `StreamKafka` descriptor
directly.
--
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]
