fix(isthmus)!: honor signed integer arithmetic options by alexandrefimov · Pull Request #1392 · substrait-io/substrait-java

alexandrefimov · 2026-10-08T16:48:11Z

Handle signed integer arithmetic preferences using native and checked Calcite operators. Plain i8/i16 operators omit overflow because their behavior depends on nullability; explicit SILENT import for these widths remains unsupported.

Export describes the Rex operator at conversion time. Later Calcite Prepare can replace it according to conformance. Option matching is case-insensitive; undeclared values are rejected before fallback selection.

Part of #1173.

BREAKING CHANGE: Unsupported integer preferences are rejected. Export adds single-value arithmetic options, except overflow on plain i8/i16 operators. Consumers without those policies must reject these plans. Undeclared values fail even with a supported fallback.

SQL CONCAT and || now emit null_handling ACCEPT_NULLS for the standard Substrait concat function. Import accepts that behavior, including as a fallback preference, instead of silently discarding options. BREAKING CHANGE: Standard concat calls with null_handling preferences that do not include ACCEPT_NULLS, unknown options, or empty preference lists are rejected. Include ACCEPT_NULLS only when null propagation is acceptable.

Use one option policy for the standard concat, like, replace, starts_with, ends_with, strpos, substring, lower, upper and initcap mappings. Export the behavior implemented by the Calcite operator and reject import preferences that do not permit it. Other extensions and unmapped option policies are unchanged. BREAKING CHANGE: The covered standard string functions reject unknown options, empty preferences and preferences that do not include the mapped Calcite behavior. Allow that behavior only when it is acceptable for the query.

Select supported signed integer arithmetic preferences from spec v0.103.0, use checked Calcite operators for overflow=ERROR, and export the native operator behavior. Keep the existing operator when options are omitted. BREAKING CHANGE: Isthmus rejects signed integer arithmetic option preferences that it cannot honor instead of silently ignoring them.

coderabbitai · 2026-10-08T16:48:23Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: substrait-io/substrait-java/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 94767447-d695-4504-8fc5-c667fee52fec

📥 Commits

Reviewing files that changed from the base of the PR and between 8e21205 and 8856a53.

📒 Files selected for processing (10)

isthmus/src/main/java/io/substrait/isthmus/expression/IntegerFunctionOptions.java
isthmus/src/main/java/io/substrait/isthmus/expression/ScalarFunctionOptionPolicy.java
isthmus/src/main/java/io/substrait/isthmus/expression/StringFunctionOptions.java
isthmus/src/test/java/io/substrait/isthmus/DdlRoundtripTest.java
isthmus/src/test/java/io/substrait/isthmus/IntegerArithmeticOptionsTest.java
isthmus/src/test/java/io/substrait/isthmus/LambdaExpressionTest.java
isthmus/src/test/java/io/substrait/isthmus/NestedExpressionsTest.java
isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java
isthmus/src/test/java/io/substrait/isthmus/VirtualTableScanTest.java
isthmus/src/test/java/io/substrait/isthmus/VirtualTableTest.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
- Integer arithmetic options are supported for addition, subtraction, multiplication, division, and modulus. Overflow preferences and division or modulus error handling are preserved when converting between Substrait and Calcite.
Tests
- Added coverage for overflow behavior, division and modulus options, mixed-width operands, and round-trip conversion. Updated arithmetic test cases to specify silent overflow where required.

Walkthrough

Integer arithmetic option handling is added to scalar function conversion. Tests cover option resolution, execution, export, and round trips. Existing arithmetic test inputs and a lambda fixture now specify silent overflow.

Changes

Integer arithmetic options

Layer / File(s)	Summary
Integer arithmetic bindings and options `isthmus/src/main/java/io/substrait/isthmus/expression/IntegerFunctionOptions.java`	Integer arithmetic bindings map supported function variants to Calcite operators. The policy emits and resolves overflow, division, and modulus options.
Option validation and converter integration `isthmus/src/main/java/io/substrait/isthmus/expression/ScalarFunctionOptionPolicy.java`, `isthmus/src/main/java/io/substrait/isthmus/expression/StringFunctionOptions.java`, `isthmus/src/main/java/io/substrait/isthmus/expression/ScalarFunctionConverter.java`	Scalar function conversion includes the integer option policy. Shared declaration-value validation is used by integer and string option handling.
Arithmetic option behavior and export `isthmus/src/test/java/io/substrait/isthmus/IntegerArithmeticOptionsTest.java`	Tests cover integer arithmetic options across widths, including overflow, division, modulus, invalid options, SQL export, and round trips.
Explicit silent-overflow test inputs `isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java`, `isthmus/src/test/java/io/substrait/isthmus/{DdlRoundtripTest,LambdaExpressionTest,NestedExpressionsTest,ProjectTest,VirtualTableScanTest,VirtualTableTest}.java`, `isthmus/src/test/resources/lambdas/lambda-with-function.json`	A shared helper adds `overflow=SILENT` to scalar invocations. Existing arithmetic test inputs and the lambda fixture now use that option.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: nielspardon

Merge Risk: ⚪ Minimal · up to 8856a

This change makes Isthmus reject signed integer arithmetic options it cannot honor and exports the native operator behavior. No merge-blocking issue was found in the supplied context.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title is a valid Conventional Commit title and clearly identifies the main change: honoring signed integer arithmetic options in Isthmus.
Description check		The description provides the rationale, implementation behavior, unsupported cases, breaking-change impact, and related issue reference. It is complete for the repository template.

Fix all pre-merge checks with AI

✨ 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

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

nielspardon

Thanks for this. One thing to settle before the rest: for TINYINT/SMALLINT the exported overflow option doesn't match what Calcite does for nullable operands.

nielspardon

Thanks, the narrow-type fix and the mixed-width export look good. A few small things left. Please also extend the BREAKING CHANGE: footer: export now writes single-value overflow and division options on integer add, subtract, multiply, divide and modulus, so a plan without options gains them on a round trip.

alexandrefimov added 4 commits October 8, 2026 18:30

fix(isthmus): validate string options against the selected operator

15983c3

nielspardon reviewed Oct 9, 2026

View reviewed changes

Comment thread isthmus/src/main/java/io/substrait/isthmus/expression/IntegerFunctionOptions.java Outdated

alexandrefimov added 4 commits October 9, 2026 10:24

refactor(isthmus): share scalar option policies across both directions

4e90106

Merge shared option policies and correct narrow integer overflow export

9ae5c9a

fix(isthmus): retain integer options after operand promotion

667c4ee

Merge main and shared scalar option policies

8e21205

nielspardon reviewed Oct 9, 2026

View reviewed changes

Merge main and address arithmetic option review

8856a53

nielspardon approved these changes Oct 9, 2026

View reviewed changes

nielspardon merged commit bc9ad04 into substrait-io:main Oct 9, 2026
16 checks passed

Navigation Menu

Repository navigation

fix(isthmus)!: honor signed integer arithmetic options - #1392

fix(isthmus)!: honor signed integer arithmetic options#1392
nielspardon merged 9 commits into
substrait-io:mainfrom
alexandrefimov:arithmetic-options-1173

alexandrefimov commented Oct 8, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 8, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

nielspardon left a comment

Uh oh!

Uh oh!

nielspardon left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

alexandrefimov commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

nielspardon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nielspardon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alexandrefimov commented Oct 8, 2026 •

edited

Loading

coderabbitai Bot commented Oct 8, 2026 •

edited

Loading