Skip to content

isthmus: window relations do not convert in either direction (LogicalWindow / ConsistentPartitionWindow) #1075

Description

@nielspardon

Summary

Isthmus converts window functions in their expression form (Expression.WindowFunctionInvocation) in both directions, but the relation form is unsupported in both directions. Calcite LogicalWindow throws on the way in, and Substrait ConsistentPartitionWindow throws on the way out.

Form Calcite → Substrait Substrait → Calcite
Expression.WindowFunctionInvocation (expression) works — RexExpressionConverter.visitOver works — ExpressionRexConverter.visitrexBuilder.makeOver
ConsistentPartitionWindow (relation) missing — no Window case in RelNodeVisitor.reverseAccept, so SubstraitRelVisitor.visitOther throws missing — no visit override, so SubstraitRelNodeConverter.visitFallback throws

Both gaps are reachable through public API, so each throws UnsupportedOperationException for callers today.

Substrait → Calcite: SubstraitRelNodeConverter.visit(ConsistentPartitionWindow)

This is the half with a concrete in-repo producer. The Spark integration builds ConsistentPartitionWindow (ToSubstraitRel.scala) and reads it back (ToLogicalPlan.scala), so a plan emitted by this project's own Spark module cannot be converted to Calcite by its own Isthmus module.

The wrinkle is that window bounds sit at different levels in the two models. Substrait puts partitions/sorts on the relation and lower_bound/upper_bound/bounds_type on each WindowRelFunction; Calcite's Window holds a list of Group, and each Group carries partition keys, order keys and bounds together. So one ConsistentPartitionWindow with N functions fans out to one Window.Group per distinct (boundsType, lowerBound, upperBound) triple, all sharing the relation's keys.

A cheaper first cut avoids LogicalWindow entirely: push the relation's partitions/sorts down into each function to synthesize Expression.WindowFunctionInvocations and emit a LogicalProject of RexOvers, reusing the already-tested makeOver path. Anything downstream that wants a real LogicalWindow can get one from ProjectToWindowRule.

Calcite → Substrait: LogicalWindow in RelNodeVisitor / SubstraitRelVisitor

RelNodeVisitor.reverseAccept has no Window case, so a LogicalWindow lands in SubstraitRelVisitor.visitOther and throws. Any caller that runs ProjectToWindowRule (or builds a Window directly) before converting hits this. Isthmus' own SQL pipeline never produces one — its Hep program is empty and RexOver stays inside a LogicalProject — which is why the gap has gone unnoticed.

Recommended target here is likewise the expression form rather than ConsistentPartitionWindow: flatten each group's aggCalls into Expression.WindowFunctionInvocations, copying that group's keys, order keys and bounds into each invocation, and emit a Substrait Project. That reuses WindowFunctionConverter (already wired in via ConverterProvider.getRexExpressionConverter), needs no fan-out because each invocation carries its own partitionBy/sort, and round-trips today because the expression form is supported in both directions.

Two traps to handle explicitly:

  • Window.constants. When Calcite builds a Window it hoists literal operands into Window.constants and rewrites them as RexInputRefs with ordinals past the input field width. Without resolving them, NTILE(4), LAG(x, 2) and LEAD(x, 1, 0) convert those literals as field references — silently wrong plans rather than an error. Resolution belongs in visit(Window), which is the only place that holds the Window.
  • Window.Group.exclude. RexWindowExclusion has no Substrait counterpart, so EXCLUDE CURRENT ROW and friends must be rejected rather than quietly dropped.

If a future change does emit ConsistentPartitionWindow from this direction instead, note that groups differing in partition or order keys cannot collapse into a single relation — that needs a stack of relations or a documented bail-out — and that emitting the relation form before the Substrait → Calcite half exists would break Calcite → Substrait → Calcite round-trips for window queries. The two halves are coupled in that direction only.

Note on the removed converter

#1014 removed WindowRelFunctionConverter, which converted a single Calcite Window.RexWinAggCall into a WindowRelFunctionInvocation. It was never instantiated by any shipped ConverterProvider, and it is not a usable starting point for the work above: WrappedWindowRelCall.getOperands() passed operands through raw with no constants resolution, and convert(...) never received the enclosing Window, so it structurally could not resolve them. Its signature needs reworking regardless. Git history retains it if useful.

Suggested order

  1. Calcite → Substrait via the expression form — self-contained, closes the public-API throw, round-trips immediately.
  2. Substrait → Calcite for ConsistentPartitionWindow — unblocks Spark-produced plans.
  3. Only then consider emitting the relation form from Calcite, if there is a reason to prefer it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions