fix(isthmus)!: write the output type the datetime extension declares

[alexandrefimov]

Datetime calls use the extension's declared result type. Casts preserve Calcite's inferred types, except for dynamic placeholders. Literals are widened directly, and interval casts use SQL qualifiers.

For DATE results, sub-day literals are truncated to whole days toward zero, and non-literal sub-day intervals are rejected. Timestamp results keep the full interval.

This is the datetime part of #1117.

BREAKING CHANGE: Consumers must handle added datetime casts. Conversion rejects DATE arithmetic with non-literal sub-day intervals, unsupported timestamp precision, and literal widening overflow.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 0a180126-2154-4deb-bdb6-cb0d8cb5f2f1

📥 Commits

Reviewing files that changed from the base of the PR and between 4c3baee and 2097256.

📒 Files selected for processing (5)

• isthmus/src/main/java/io/substrait/isthmus/expression/ScalarFunctionConverter.java
• isthmus/src/test/java/io/substrait/isthmus/DatetimeBindingRegressionTest.java
• isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java
• isthmus/src/test/java/io/substrait/isthmus/PrecisionTimestampDatetimeAdditionTest.java
• isthmus/src/test/java/io/substrait/isthmus/PrecisionTimestampDatetimeSubtractionTest.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

• Bug Fixes
  • Improved date and timestamp arithmetic conversions to preserve expected precision and return results in the appropriate type.
  • Corrected datetime extraction results when the function’s declared type differs from the type expected by the query.
  • Improved datetime comparisons across timestamp precisions, preserving operand precision and handling timestamp literals consistently.
  • DATE arithmetic with sub-day interval literals now truncates intervals toward zero to whole days; unsupported non-literal interval qualifiers are reported.
  • Improved interval and timestamp conversions to preserve signed values, subsecond precision, and nulls, and to report overflow or unsupported precision.
  • Improved SQL casts for day-time and year-month intervals.

Walkthrough

Datetime scalar function conversion now resolves datetime-extension output types from argument types and adjusts supported argument precisions. Date-output addition and subtraction handle day interval operands, including truncation of sub-day literal fields. Tests cover datetime comparisons, expression types and casts, interval arithmetic, and conversion round trips.

Changes

Datetime conversion

Layer / File(s)	Summary
Datetime binding and precision handling isthmus/src/main/java/io/substrait/isthmus/expression/ScalarFunctionConverter.java, isthmus/src/main/java/io/substrait/isthmus/SimpleExtensionToSqlOperator.java, isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlDialect.java, isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java, isthmus/src/test/java/io/substrait/isthmus/PrecisionTimestampDatetimeAdditionTest.java, isthmus/src/test/java/io/substrait/isthmus/PrecisionTimestampDatetimeSubtractionTest.java, isthmus/src/test/java/io/substrait/isthmus/DatetimeBindingRegressionTest.java, isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java	Datetime binding resolves output types from expression arguments and widens supported argument precisions. Resolved invocations are cast when their output type differs from Calcite’s inferred type, except for placeholder return types. The SQL dialect uses interval qualifiers in cast specifications. Tests cover extraction, comparisons, precision widening, timestamp and interval conversions, and configured precision limits.
Date interval arithmetic isthmus/src/main/java/io/substrait/isthmus/expression/ScalarFunctionConverter.java, isthmus/src/test/java/io/substrait/isthmus/DateIntervalArithmeticTest.java, isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java	Date-output addition and subtraction handle day-qualified interval operands. Literal intervals have sub-day fields truncated; unsupported non-literal intervals throw UnsupportedOperationException. Tests cover interval qualifiers, timestamp-result operations, round trips, and reverse-conversion output types.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: nielspardon

Merge Risk: ⚪ Minimal · up to 20972

No concrete merge-blocking issue is identified. The datetime conversion changes appear ready to merge subject to normal build and test checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 20972

The change adds datetime casts and new rejection cases that consumers must handle. The inspected conversion path does not introduce privileged operations or external execution, but downstream service exposure and deployment compatibility remain unestablished.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — A caller controlling datetime expressions can affect generated plan types, interval values, and conversion failures. The inspected change operates on library expression objects; it does not establish additional tenant, credential, data-store, or environment authority.

Trust Boundaries and Controls

• observed — The inspected boundary translates Calcite calls into typed Substrait expressions. Extension URN and function-key checks select conversion policy, not authentication or authorization. The changed helpers emit expression objects rather than executing SQL text or invoking an external service.

Resilience and Maintainability Implications

• observed — The inspected normalization helpers construct new argument lists and expressions without modifying the supplied argument list. Conversion-time rejection occurs before returning a result, and unresolved declarations return an invocation using the original arguments. These helpers do not expose an intermediate shared-state update requiring rollback; containment of propagated exceptions by downstream services remains outside the inspected scope.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 10 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: using the datetime extension's declared output type. It follows the required Conventional Commit format and marks the breaking change.
Description check	✅ Passed	The description provides the rationale, summarizes the datetime behavior and limitations, references the related issue, and includes a BREAKING CHANGE footer. It satisfies the repository template requ…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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.}

[nielspardon]

Two things before the rest: restrict the operand widening to the declarations whose P reaches the result, and decide what date ± interval_day should do with a sub-day interval. Everything else is smaller and can wait for the next round.

[nielspardon]

Thanks, the DATE handling is exactly it. Below is the next round. Please also change #1117 in the PR body to the full issue URL, because semantic-release renders a bare reference as "closes #1117" in the changelog while the issue stays open.

[nielspardon]

Thanks, all five are fixed. The last few are small.

[alexandrefimov]

Fixed all five and added round-trip tests.

[nielspardon]

@coderabbitai resume

[coderabbitai]

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

[nielspardon]

LGTM