fix(isthmus)!: build a character literal from the type its conversion produced

[alexandrefimov]

LiteralConverter.convert derives the Substrait type twice. Once through typeConverter.toSubstrait(resultType), which consults the UserTypeMapper, and again in the switch over resultType.getSqlTypeName(), which does not — and the second derivation is the one the literal gets. So a mapped character column produced a schema of the mapped type and literals of the unmapped one, and VirtualTableScan rejected the relation:

SELECT * FROM (VALUES ('a'), ('b')) AS v(c)     -- with CHAR/VARCHAR mapped to string
-- Row field type (FixedChar{nullable=false, length=1})
--   does not match schema field type (Str{nullable=false})

A null literal takes its type straight from the first derivation, so it already agreed; only rows carrying a value diverged. That asymmetry is what the nullAndNonNullLiteralsCarryTheSameMappedType test pins.

The character branches now build from the type already derived, which covers the three forms a Calcite character type can map to — fixedchar, varchar and string. It also removes a copy of the PRECISION_NOT_SPECIFIED rule that TypeConverter already applies — the VARCHAR one, which is the only arm that had it. CHAR had none, so a reflective schema's char or Character column, which carries no precision at all, derived a fixedchar<-1>: outside the [1..2147483647] the spec allows, and a width the check below then measured every value against, the empty string included. That arm now reads the unspecified precision as Calcite's default of 1, and each rule lives in one place.

A fixedchar literal needed one thing more: it carries no length of its own, since Expression.FixedCharLiteral derives the type from its text. Building one from the declared type therefore means padding the text to the declared width, which is also what CHAR(n) means — 'a' in a CHAR(3) is 'a ' — and what padRightIfNeeded already does for BINARY a few lines down. A value longer than the width it is declared as is rejected rather than truncated. The width is counted in characters, as the spec gives it -- a FIXEDCHAR<L> is L characters where a string is a count of UTF-8 bytes -- so one astral character fills a fixedchar<1>.

That half fixes a case needing no mapper at all. A LogicalValues row field wider than its literal — the shape #1064 was reported with — was still rejected for character types: a CHAR(3) field holding 'a' produced Row field type (FixedChar{length=1}) does not match schema field type (FixedChar{length=3}). Isthmus' own SQL path does not reach it, because Calcite pads the literal itself; a plan arriving from another planner does.

What this does not cover, deliberately. A mapper that maps some other family still diverges the same way — mapping INTEGER to i64 gives Row field type (I32) does not match schema field type (I64). Closing that generally means dispatching every branch on the Substrait type rather than the Calcite type name, which is a much larger change to this method, and it still would not reach a genuine Type.UserDefined: there is no way to build a literal of one from a RexLiteral without knowing its encoding, so that needs a literal-side hook the interface does not have. The character family is the part that can be closed without deciding either of those. A mapping outside it leaves the literal in the form Calcite declares, which is what it converted to before this change; under a virtual table that still puts the schema and its rows out of step, and closing it needs the literal-side hook above.

Worth saying, since it bears on whether this use is meant to be supported at all: UserTypeMapper.toSubstrait returns a Type, so mapping to a built-in is expressible, while its reverse takes a Type.UserDefined. The consumer this came from maps Impala's string — reaching Calcite as VARCHAR(2147483647) — onto Substrait's string, which is a natural reading of the forward signature. That reading is the intended one: TypeConverter consults the mapper before its own switch precisely because a mapper may re-use a SqlTypeName, so mapping a Calcite built-in onto a Substrait built-in is what the forward direction offers. The reverse lags behind it — toCalcite takes a Type.UserDefined, so a mapped built-in has no hook to come back through — which is #1219.

A negative width is refused where the padding happens: a mapper hands the literal a Substrait type directly, so nothing between it and this method holds its width to what a fixedchar can declare. It threw before as well, but reported the value as longer than the fixedchar<-5> it was declared as, rather than the width as the problem.

The floor stops at the negatives rather than at the [1..2147483647] type_classes.md gives L. Calcite types '' as a CHAR(0) and its DDL parser takes a CHAR(0) column, so a fixedchar<0> reaches a plan from ordinary SQL — on origin/main (934a60e) SELECT '' FROM t, WHERE '' = '', CHAR_LENGTH('') and '' || 'x' all convert to one — and the bound itself is under discussion in substrait-io/substrait#1199, where dialect_schema.yaml allows a max_length of 0 for the same three types. A zero width therefore converts as it does today, rather than being refused now and legalized again if that issue lands the other way.

Closes #1170.

BREAKING CHANGE: a Calcite CHAR(n) literal converts to a fixedchar<n> whose text is padded to the declared width, where it previously carried the unpadded text and so a fixedchar of the text's own length. A character value longer than the width it is declared as is rejected rather than converted.

[nielspardon]

Whether a mapper returning a non-character type should fall back to the Calcite-derived literal instead of throwing is worth settling first — it converted before this PR, and the same question decides #1190. Worth marking this ! either way, since the padding changes the plans existing consumers get. I have more on the padding, the width check and the tests, but let's start here.

[nielspardon]

The fallback reads well. Three left, and the first is also a correction to the description: TypeConverter does not apply the PRECISION_NOT_SPECIFIED rule for CHAR.

[nielspardon]

Two left, both on the width guard.

[nielspardon]

No need — leave the mapper unrestricted. TypeConverter consults it before its own switch, with the call site saying it does so because mappers "may re-use SqlTypeNames"; were the mapper restricted to user-defined types, that ordering would buy nothing, since such a type's SqlTypeName is not one the switch handles and would fall through to default either way. So mapping a Calcite built-in onto a Substrait built-in is the intended forward contract, and the fallback here is the right shape.

What lags is the reverse, which is now #1219: toCalcite takes only a Type.UserDefined and ToRelDataType consults it from that one visit, so a mapped built-in has no hook to come back through. #1190 covers the other literal families.

[alexandrefimov]

Left unrestricted, and the description carries that now rather than the question.

[nielspardon]

Filed the floor as a spec issue — substrait-io/substrait#1199. type_classes.md puts L at 1 or more, but dialect_schema.yaml allows max_length: 0 for the same three types, so the bound this PR keys to is stated inconsistently. Worth saying in the body that mapping a CHAR(0) to string is what the spec allows today rather than the settled answer, since #1199 may legalize a zero width instead.

One consequence the footer does not cover: an empty character type no longer round-trips. CHAR(0) becomes a string, and ToRelDataType.visit(Type.Str) gives back an unparameterised VARCHAR, where before it came back as a CHAR(0). Worth a clause there, since the footer describes the forward conversion only.

[nielspardon]

Drop the CHAR(0) → string commit and narrow the floor to length < 0, leaving fixedchar<0> as it was pending substrait-io/substrait#1199. That reverses my round-three ask: the floor was right about negatives and wrong to include zero.

type_classes.md's [1..2147483647] is the only thing asserting the floor. dialect_schema.yaml allows max_length: 0 for FIXED_CHAR independently of it, and Calcite produces CHAR(0) from the four SQL forms you found — so refusing it is a breaking change #1199 may reverse, and undoing it would take a second one. Dropping the mapping also drops two costs it carried: the lost round trip, and re-binding string functions onto their str impls, since 16 functions in functions_string.yaml have a fixedchar<L1> impl distinct from their string one.

The negatives half stands on its own — unambiguous under any reading of the spec, and it fixes a message that reported the value's length where the width was the problem. That also answers the split question: with the CHAR(0) half gone there is nothing left to split.

[alexandrefimov]

The description no longer presents the zero width as settled -- it says the bound is #1199's and that a fixedchar<0> converts as it does today -- and the footer is down to the padding and the refusal, so the round-trip clause has nothing left to describe.

One thing to weigh for #1199: dialect_schema.yaml's max_length is a dialect's ceiling rather than a width the spec permits, and its minimum: 0 reads as "this dialect does not support the type" about as easily as "a zero width is legal". The four SQL forms look like the stronger half of the case to me -- Calcite emits CHAR(0) whatever type_classes.md says, so any consumer reading a plan isthmus produced has to hold one either way.

[nielspardon]

LGTM, thanks