fix(isthmus)!: use schema types for virtual table literals

[alexandrefimov]

Summary

• convert LogicalValues tuple literals using the complete row-schema field type;
• preserve the existing literal-based behavior for other expression conversions;
• add a regression test for a TINYINT tuple literal in an INTEGER field.

VirtualTableScan requires row field types to exactly match its schema. Calcite
may infer a narrower type for a tuple literal than for the corresponding
LogicalValues row field — Values.assertRowType() only asserts that the field
type canAssignFrom the literal type, so a TINYINT literal is a legal member of
an INTEGER row. Previously Isthmus copied only schema nullability, so the
conversion could construct an I8 row under an I32 schema and fail its own
validation.

LiteralConverter.convert now takes the schema RelDataType, and derives
nullability from it rather than from a separate flag. Existing callers continue to
derive the result type from the literal itself.

Fixes #1064

BREAKING CHANGE: io.substrait.isthmus.expression.LiteralConverter#convert(RexLiteral, boolean) is removed. Use convert(RexLiteral, RelDataType), which takes nullability from the supplied Calcite type; callers needing a nullability other than the literal's own should widen the type with RelDataTypeFactory#createTypeWithNullability first.

[alexandrefimov]

@nielspardon would you mind taking a look at this one? Small isthmus fix for #1064: VirtualTableScan requires row field types to match its schema exactly, but Calcite can infer a narrower type for a tuple literal than for the corresponding LogicalValues row field — so the conversion could build an I8 row under an I32 schema and then fail its own validation.

[vbarua]

I would advocate for getting rid of this method entirely actually.

It's not used internally after your changes, and any user that wants to set the nullability explicitly on the Calcite type can use the RelDataTypeFactory#createTypeWithNullability to set it, and then pass it to the new method below.

[alexandrefimov]

Removed. It has been public since #684, so the PR is now fix(isthmus)! with a footer pointing at convert(RexLiteral, RelDataType).

[vbarua]

To confirm two things.

In your test, you construct a LogicalValue containing literals with different but compatible(?) types than the row type of the LogicalValue relation, and that ends up being valid because Calcite checks if the type can be assigned to value?
https://github.com/apache/calcite/blob/052a35015c56db2927727afd7e08ed7a18a26ace/core/src/main/java/org/apache/calcite/rel/core/Values.java#L169-L190. The issue that you've identified is that Calcite isn't as strict as Substrait, so when we convert from Calcite we can't rely on the literal types and need to override them with the row type?

In Substrait, we assume that all literals in VirtualTables have the same types as given in the schema. When we convert from Substrait to Calcite, do the tuple values in the Calcite LogicalValues relation this, or is that conversion also potentially lossy?

[alexandrefimov]

@vbarua both confirmed.

On strictness — the asymmetry is canAssignFrom against VirtualTableScan's @Value.Check, which requires rowFieldType.equals(schemaFieldType), nullability included.

The reverse direction isn't lossy, and structurally so: that same check makes a mismatched VirtualTableScan unconstructible, so nothing reaches the converter to lose. visit(VirtualTableScan) then derives the row type and every literal from the same TypeConverter over the same Substrait types. Checked i8, i32, i64, decimal(10,2), varchar(5), char(3) and string — every literal type is equal to its row field type, not merely assignable.

Both inline suggestions applied; the javadoc now points at createTypeWithNullability for callers wanting a nullability other than the literal's own. That does make it an API-visible removal — convert(RexLiteral, boolean) has been public since #684, so by the standard just applied in #1014 it's breaking. Retitled fix(isthmus)! with a footer; releaseRules maps that to a minor bump. Say the word if you'd rather split the removal out and keep this one non-breaking.

[alexandrefimov]

@vbarua ping when you get a chance. The overload is gone, so the PR is now fix(isthmus)! with a footer pointing at convert(RexLiteral, RelDataType) — that thread is left open because the call on removing a public API is yours.