UserTypeMapper.toSubstrait returns any Type, and TypeConverter consults it before its own switch precisely so a mapper can claim a SqlTypeName the switch already handles — the call site says so:
private Type toSubstrait(RelDataType type, List<String> names) {
// Check for user mapped types first as they may re-use SqlTypeNames
Type userType = userTypeMapper.toSubstrait(type);
Mapping a Calcite built-in onto a Substrait built-in is therefore part of the forward contract rather than an accident of the signature: were the mapper restricted to user-defined types, checking it first would buy nothing, since a user-defined type's SqlTypeName is not one the switch handles and would fall through to default either way.
The reverse direction does not follow. toCalcite takes a Type.UserDefined, and TypeConverter.ToRelDataType calls it from exactly one place, visit(Type.UserDefined); every other visit builds a Calcite type from the factory with no hook. So a mapped built-in has no way back:
Calcite CHAR(1) -> mapper -> Type.Str -> ToRelDataType.visit(Type.Str) -> VARCHAR, PRECISION_NOT_SPECIFIED
A column that went in as CHAR(1) comes back as an unparameterised VARCHAR, and the mapper that made the forward decision is never asked. The consumer #1170 was filed for maps Impala's string — which reaches Calcite as VARCHAR(2147483647) — onto Substrait's string, so this is the shape the forward direction is actually used in.
Two ways to close it, and they are not equivalent. Widening toCalcite to take a Type and consulting it from every ToRelDataType.visit keeps the forward contract and makes the round trip the mapper's own responsibility. Narrowing toSubstrait to Type.UserDefined would make the two signatures agree instead, but it contradicts the mapper-first ordering above and removes the use #1170 was filed for. The interface's class Javadoc — "Defines conversion of user-defined types between Substrait and Calcite" — reads as though the second were already the contract, so it needs updating either way.
Found while reviewing #1171, whose description raises the same asymmetry from the forward side. #1190 covers the separate problem that most literal families ignore the mapper's answer. Checked on main @ 934a60e.
UserTypeMapper.toSubstraitreturns anyType, andTypeConverterconsults it before its own switch precisely so a mapper can claim aSqlTypeNamethe switch already handles — the call site says so:Mapping a Calcite built-in onto a Substrait built-in is therefore part of the forward contract rather than an accident of the signature: were the mapper restricted to user-defined types, checking it first would buy nothing, since a user-defined type's
SqlTypeNameis not one the switch handles and would fall through todefaulteither way.The reverse direction does not follow.
toCalcitetakes aType.UserDefined, andTypeConverter.ToRelDataTypecalls it from exactly one place,visit(Type.UserDefined); every othervisitbuilds a Calcite type from the factory with no hook. So a mapped built-in has no way back:A column that went in as
CHAR(1)comes back as an unparameterisedVARCHAR, and the mapper that made the forward decision is never asked. The consumer #1170 was filed for maps Impala'sstring— which reaches Calcite asVARCHAR(2147483647)— onto Substrait'sstring, so this is the shape the forward direction is actually used in.Two ways to close it, and they are not equivalent. Widening
toCalciteto take aTypeand consulting it from everyToRelDataType.visitkeeps the forward contract and makes the round trip the mapper's own responsibility. NarrowingtoSubstraittoType.UserDefinedwould make the two signatures agree instead, but it contradicts the mapper-first ordering above and removes the use #1170 was filed for. The interface's class Javadoc — "Defines conversion of user-defined types between Substrait and Calcite" — reads as though the second were already the contract, so it needs updating either way.Found while reviewing #1171, whose description raises the same asymmetry from the forward side. #1190 covers the separate problem that most literal families ignore the mapper's answer. Checked on
main@ 934a60e.