Skip to content

feat(isthmus): map RIGHTSHIFT to Substrait shift_right#986

Merged
nielspardon merged 1 commit into
substrait-io:mainfrom
nielspardon:feat/isthmus-right-shift
Jul 22, 2026
Merged

feat(isthmus): map RIGHTSHIFT to Substrait shift_right#986
nielspardon merged 1 commit into
substrait-io:mainfrom
nielspardon:feat/isthmus-right-shift

Conversation

@nielspardon

@nielspardon nielspardon commented Jul 3, 2026

Copy link
Copy Markdown
Member

What

Adds a mapping so a right-shift can be converted to the Substrait shift_right function. Calcite defines the << operator (BIT_LEFT_SHIFT) and the LEFTSHIFT function — both already mapped to Substrait shift_left — but has no right-shift counterpart at all. This PR defines an Isthmus RIGHTSHIFT(value, shift) SqlFunction (mirroring Calcite's LEFTSHIFT), registers it in SubstraitOperatorTable, and maps it to shift_right.

Why

shift_right exists in functions_arithmetic.yaml (i32/i64, same shape as shift_left) but was unreachable from SQL because no Calcite operator produced it.

Scope

  • Covers the function-call spelling RIGHTSHIFT(x, n). Round-trips to Substrait shift_right and back.
  • The >> infix operator is intentionally out of scope. Calcite 1.42 has no >> token, no SqlKind, and no operator (only <<). Supporting >> requires isthmus to generate its own parser: Calcite exposes the needed extension hooks (binaryOperatorsTokens + extraBinaryExpressions, the same mechanism calcite-babel uses for the :: infix cast), but isthmus currently consumes the prebuilt SqlDdlParserImpl and has no parser codegen. Tracking that as a separate follow-up; the cleanest long-term fix is likely to upstream BIT_RIGHT_SHIFT into Calcite core (a genuine gap, given it already has <<), which is being pursued in apache/calcite#5073 — once that lands, this reduces to a one-line mapping.

Testing

  • New round-trip test in ArithmeticFunctionTest (i8/i16/i32/i64), symmetric to the existing leftshift test.
  • Full :isthmus:test suite passes locally.

🤖 Generated with AI

Calcite defines the << operator (BIT_LEFT_SHIFT) and the LEFTSHIFT function, both mapped to Substrait shift_left, but has no right-shift counterpart. Define an Isthmus RIGHTSHIFT(value, shift) SqlFunction mirroring Calcite's LEFTSHIFT, register it in SubstraitOperatorTable so it validates, and map it to the Substrait shift_right function.

This covers the function-call spelling RIGHTSHIFT(x, n). The >> infix operator is intentionally out of scope here: Calcite has no >> token, so supporting it requires generating a custom parser.
@nielspardon
nielspardon marked this pull request as ready for review July 6, 2026 12:00

@vbarua vbarua left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me.

@nielspardon
nielspardon merged commit 6a44814 into substrait-io:main Jul 22, 2026
16 checks passed
@nielspardon
nielspardon deleted the feat/isthmus-right-shift branch July 22, 2026 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants