[anasik]

Calcite → Substrait:

• HASH_DISTRIBUTED → ScatterExchange
• BROADCAST_DISTRIBUTED → BroadcastExchange
• ROUND_ROBIN_DISTRIBUTED → RoundRobinExchange with exact = true, since Calcite defines it as "records are assigned to instances in turn"
• RANDOM_DISTRIBUTED → RoundRobinExchange with exact = false, and SINGLETON → SingleBucketExchange with a constant bucket and a partition count of 1, as proposed in Questions about exchangeKind of ExchangeRel substrait#519
• A Calcite plan carries no partition count or targets, so the count is left unset (0) and targets empty

Substrait → Calcite:

• ScatterExchange → HASH_DISTRIBUTED, BroadcastExchange → BROADCAST_DISTRIBUTED, RoundRobinExchange → ROUND_ROBIN_DISTRIBUTED
• Partition count, targets and round-robin exactness are dropped, since RelDistribution has nowhere to put them

Not supported, rejected instead of converted lossily:

• RANGE_DISTRIBUTED: RelDistribution carries no range boundaries, so the range-bucket expression from Questions about exchangeKind of ExchangeRel substrait#519 can't be built
• SortExchange: no Substrait equivalent for its per-partition sort
• SingleBucketExchange and MultiBucketExchange → Calcite: no RelDistribution gives what they ask for (multi-bucket can duplicate or drop rows)
• A ScatterExchange on a nested field, or on fields not in ascending order: Calcite sorts hash keys, which would change partition alignment between exchanges

SqlKindFromRel also returned OTHER_DDL for exchanges. Calcite's RelRoot.project skips applying output names for DDL kinds, so an exchange root could lose its output names; exchanges now return SELECT like the other relations that only pass rows through.

Closes #153

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: substrait-io/substrait-java
• Branch: main
• File: .coderabbit.yaml
• Commit: d35cc52
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2c2d4f3f-441a-402a-8829-75b061546035

Commits

Reviewing files that changed from the base of the PR and between d35cc52 and 7dac776.

Files selected for processing: 4

• isthmus/src/main/java/io/substrait/isthmus/SqlKindFromRel.java
• isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java
• isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java
• isthmus/src/test/java/io/substrait/isthmus/SubstraitRelNodeConverterTest.java

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Summary

Summary by CodeRabbit

• New Features
  • Added conversion support for hash, broadcast, round-robin, random, and singleton exchanges between Substrait and Calcite.
  • Exchange relations are now classified as queries.
• Limitations
  • Sort exchanges and unsupported distribution or scatter-field shapes remain unsupported.

Isthmus now converts supported exchanges between Calcite and Substrait. The conversion handles hash, broadcast, round-robin, random, and singleton distributions, with explicit rejection of unsupported cases. Substrait exchange relations now map to query kind.

Changes: Exchange conversion

Layer / File(s)	Summary
Exchange mappings and validation isthmus/src/main/java/io/substrait/isthmus/SubstraitRelVisitor.java, isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java, isthmus/src/main/java/io/substrait/isthmus/SqlKindFromRel.java, isthmus/src/test/java/io/substrait/isthmus/SubstraitRelNodeConverterTest.java	Calcite hash, broadcast, round-robin, random, and singleton distributions map to Substrait exchanges. Supported Substrait exchanges map to Calcite distributions; scatter keys must be top-level struct fields in ascending index order. Sort, range, and other unsupported distributions are rejected. Tests cover conversion results, rejected inputs, and preserved root output names.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant CalciteRel
  participant SubstraitRelVisitor
  participant SubstraitExchange
  participant SubstraitRelNodeConverter
  CalciteRel->>SubstraitRelVisitor: Convert Calcite exchange distribution
  SubstraitRelVisitor->>SubstraitExchange: Create mapped exchange relation
  SubstraitExchange->>SubstraitRelNodeConverter: Convert Substrait exchange
  SubstraitRelNodeConverter->>CalciteRel: Apply Calcite distribution

Loading

Suggested reviewers: cht42, jacques-n, nielspardon

Merge Risk: Minimal · up to 7dac7

No actionable issue remains from this review; the exchange conversion is ready to merge after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 3.70% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title uses a valid Conventional Commit format and clearly describes the conversion of exchange relations between Calcite and Substrait.
Description check		The description provides the rationale, supported mappings, rejected conversions, data-loss behavior, and linked issue context required by the template.
Linked Issues check		Issue [#153] requires Isthmus support for Calcite LogicalExchange. SubstraitRelVisitor now converts Calcite hash, broadcast, round-robin, random, singleton, and rejected unsupported distributions.…
Out of Scope Changes check		The changes stay within exchange conversion support for [#153]. The SqlKindFromRel change preserves output names for exchange roots. The converter changes and exchange tests support the requested re…

Full details: Linked Issues check

Explanation

Issue [#153] requires Isthmus support for Calcite LogicalExchange. SubstraitRelVisitor now converts Calcite hash, broadcast, round-robin, random, singleton, and rejected unsupported distributions. SubstraitRelNodeConverter converts supported Substrait exchanges back to Calcite and rejects lossy cases. SubstraitRelNodeConverterTest covers both directions and unsupported cases. SqlKindFromRel returns SELECT for exchange relations, so exchange root output names remain available.

Full details: Out of Scope Changes check

Explanation

The changes stay within exchange conversion support for [#153]. The SqlKindFromRel change preserves output names for exchange roots. The converter changes and exchange tests support the requested relation handling. No unrelated change is established by the supplied PR evidence.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}