[Isthmus] Support LogicalExchange relation

[whutjs]

Isthmus cannot handle LogicalExchange relation now:

Exception in thread "main" java.lang.UnsupportedOperationException: Unable to handle node: rel#6:LogicalExchange.NONE.[](input=LogicalTableScan#0,distribution=hash[2, 4])
	at io.substrait.isthmus.SubstraitRelVisitor.visitOther(SubstraitRelVisitor.java:359)
	at io.substrait.isthmus.SubstraitRelVisitor.visitOther(SubstraitRelVisitor.java:61)
	at io.substrait.isthmus.RelNodeVisitor.visit(RelNodeVisitor.java:68)
	at io.substrait.isthmus.SubstraitRelVisitor.visit(SubstraitRelVisitor.java:349)
	at io.substrait.isthmus.SubstraitRelVisitor.visit(SubstraitRelVisitor.java:61)
	at io.substrait.isthmus.RelNodeVisitor.reverseAccept(RelNodeVisitor.java:111)

[vbarua]

Currently, the Substrait spec doesn't have a relation which would correspond to a Calcite LogicalExchange, so we don't/can't convert it.

What is your use case for this?

[whutjs]

@vbarua Isn't exchange-operator corresponded to it?

[vbarua]

Huh, I don't how I missed that. It's in the proto spec as well.

It's odd that it's not included as one of the possible rels in

message Rel {
  oneof rel_type {

I think it would need to be included in order for us to return it is as a result in the visitor

public class SubstraitRelVisitor extends RelNodeVisitor<Rel, RuntimeException>

I suspect that might be an oversight the spec. I'll ask the substrait folks about it.

[vbarua]

The answer appears to be that no one has needed it until now.

Given that it's already (mostly) in the spec, I think the steps for adding support for it in Isthmus would be:

1. Add ExchangeRel as an option to the Rel message
2. Add a POJO (Plain Old Java Object) representation of the ExhangeRel in io.substrait.relation
3. Update the Proto <-> POJO converters: ProtoRelConverter and RelProtoConverter
4. Update the POJO <-> Calcite converter: SubstraitRelConverter and SubstraitRelNodeConverter

[whutjs]

@vbarua hello, may I ask the status of this issue? Does community plan to support it?

[vbarua]

The community is open to having this be in substrait-java and Isthmus, especially as it's already in the core spec. This would require the work above, and someone with the motivation and time to do it.

I'm happy to provide pointers and reviews if you wanted to implement it yourself.

[whutjs]

@vbarua thanks for reply. Then I think I might take this issue myself because we need this feature in our scenario. By the way, is there an example or detail explanation about how exchange relation is used in substrait? I find the doc is a little simple for newbie. For example, what are the Single Bucket and Multi Bucket exchange types actually? In Velox, there are only two types of exchange: gather or repartition.

[vbarua]

By the way, is there an example or detail explanation about how exchange relation is used in substrait?

The only other source of info I know of would the protos themselves
https://github.com/substrait-io/substrait/blob/e486775009c40e1a010dc54776b976b1eddea7ca/proto/substrait/algebra.proto#L287-L341
but those also look pretty sparse.

You might be able to get more information by asking on the mailing list and/or Slack channel.
https://substrait.io/community/#get-in-touch

I will add, the Exchange relation as defined in the spec has a lot of types of exchanges. I wouldn't make it a requirement of any changes made to add support for all of them. Partial support for a subset of the exchanges would be perfectly acceptable, and allow other people to add support as needed.

[whutjs]

@vbarua thank you, I will look into it.