fix(isthmus)!: keep a struct's declared field nullability in Calcite

[alexandrefimov]

A Substrait struct declares its own nullability and its fields' separately. TypeConverter applied the struct's with createTypeWithNullability, which Calcite documents as widening a record's fields along with the record, so struct?<i32, fp64> came back as struct?<i32?, fp64?>.

enforceTypeWithNullability canonizes the result the same way and leaves each field as declared. A computed field inside a nullable struct now converts back instead of reporting that the row type cannot say what the schema said.

Closes #1154

BREAKING CHANGE: Substrait-to-Calcite conversion keeps a struct field's declared nullability, so a required field inside a nullable struct stays NOT NULL.

Summary by CodeRabbit

• Bug Fixes
  • Preserved the declared nullability of record fields during type conversion, including required fields within nullable records.
  • Corrected schema handling so required columns remain non-nullable.

[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: defaults

Review profile: CHILL

Plan: Advanced

Run ID: dabe3c4a-c15c-4916-af54-79abd6e38620

📥 Commits

Reviewing files that changed from the base of the PR and between ade72bc and 3e6b7b1.

📒 Files selected for processing (4)

• isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java
• isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java
• isthmus/src/test/java/io/substrait/isthmus/SchemaCollectorTest.java
• isthmus/src/test/java/io/substrait/isthmus/VirtualTableScanTest.java

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The change uses Calcite’s enforceTypeWithNullability when converting Substrait types. Tests verify that nullable structs retain their fields’ declared nullability through Calcite conversion and back.

Changes

Struct nullability conversion

Layer / File(s)	Summary
Apply struct nullability without widening fields isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java, isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java	ToRelDataType.n now uses enforceTypeWithNullability. A direct conversion test checks that a nullable struct retains the declared nullability of each field and round-trips to the original Substrait type.
Verify nullability in schema conversion isthmus/src/test/java/io/substrait/isthmus/SchemaCollectorTest.java, isthmus/src/test/java/io/substrait/isthmus/VirtualTableScanTest.java	Tests expect required fields to remain non-nullable in collected and scanned schemas. A virtual table scan test now expects conversion back to match the original schema instead of throwing UnsupportedOperationException.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: nielspardon

Merge Risk: ⚪ Minimal · up to 3e6b7

No actionable issue remains; this change is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 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	✅ Passed	The title is a concise Conventional Commit title. It clearly describes the main change: preserving declared struct field nullability in Calcite, and it correctly marks the change as breaking.
Description check	✅ Passed	The description provides the rationale, identifies the affected API behavior, references the related issue, and includes a BREAKING CHANGE footer. It matches the repository template requirements.
Linked Issues check	✅ Passed	PR #1317 meets the coding requirements in issue #1154. TypeConverter.ToRelDataType.n now uses enforceTypeWithNullability, so a nullable struct keeps each field's declared nullability. `CalciteType…
Out of Scope Changes check	✅ Passed	The implementation change is limited to struct nullability handling in TypeConverter. The changed tests verify the linked issue behavior and its affected virtual-table and computed-field paths. The …

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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