Context
#1166 gave ConverterProvider.Builder a callConverters(UnaryOperator<List<CallConverter>>) transform, so a caller can place its own CallConverter anywhere in the list. That solves placement. It does not let a caller reuse a built-in converter, because every built-in except one fuses its operator predicate into the body of a public static final value rather than exposing the conversion itself.
The problem
Each built-in opens by testing call.getKind() and returning null otherwise, and the body is unreachable any other way — these are constants, not methods:
CAST — switch (call.getKind()) { case CAST: … case SAFE_CAST: … default: return null; } (CallConverters.java:54)CASE — if (call.getKind() != SqlKind.CASE) { return null; } (:215)REINTERPRET (:101), ROW (:162), CREATE_SEARCH_CONV (:257) — same shape
A dialect that spells the same operation with a different SqlKind therefore cannot delegate. CallConverters.CASE.apply(call, visitor) returns null for an operator carrying SqlKind.OTHER, and there is no convertCase(...) to call instead. The caller's only options are to reconstruct the RexCall against SqlStdOperatorTable.CASE before delegating, or to copy the body.
Copying is the part that will bite. CAST's body carries an invariant documented at CallConverters.java:41-45: a nullability-only cast over a non-null literal is how ExpressionRexConverter encodes a nullable Substrait literal, so it must fold back to a literal rather than an Expression.Cast. A downstream copy of that body is a fork of an internal encoding — when the encoding changes, every copy silently produces plans that no longer round-trip, and nothing in this repository can detect it. CASE's body likewise carries the odd-operand validation and the query-order requirement of the IfClause loop.
The precedent is already in the same file
EXECUTION_CONTEXT_VARIABLE handles exactly this situation and its javadoc says so (CallConverters.java:273):
Matching is done on operator identity (these are niladic SqlKind.OTHER_FUNCTION functions with no dedicated SqlKind).
"An operator with no dedicated SqlKind" is precisely the dialect case. One converter in the set already matches on operator identity instead of kind; the rest cannot be asked to.
Proposal
Separate the predicate from the conversion, so a dialect supplies only what differs:
- factories that take the predicate —
CallConverters.cast(TypeConverter, Predicate<RexCall>), CallConverters.caseWhen(Predicate<RexCall>) — with the existing constants becoming the kind-based instances of those, or
- the bodies as callable helpers a caller's own
CallConverter can invoke after its own matching.
Either keeps one copy of the CAST literal-folding and CASE validation logic in this repository and reduces a dialect converter to its predicate.
Notes
Independent of #1166 — the transform decides where a converter sits, not whether the built-in conversion can be reached. Adjacent to but not covered by #1012, which is about the function-mapping layer (Sig tables, the matcher, reverse lookup) rather than these non-function converters.
Context
#1166 gave
ConverterProvider.BuilderacallConverters(UnaryOperator<List<CallConverter>>)transform, so a caller can place its ownCallConverteranywhere in the list. That solves placement. It does not let a caller reuse a built-in converter, because every built-in except one fuses its operator predicate into the body of apublic static finalvalue rather than exposing the conversion itself.The problem
Each built-in opens by testing
call.getKind()and returningnullotherwise, and the body is unreachable any other way — these are constants, not methods:CAST—switch (call.getKind()) { case CAST: … case SAFE_CAST: … default: return null; }(CallConverters.java:54)CASE—if (call.getKind() != SqlKind.CASE) { return null; }(:215)REINTERPRET(:101),ROW(:162),CREATE_SEARCH_CONV(:257) — same shapeA dialect that spells the same operation with a different
SqlKindtherefore cannot delegate.CallConverters.CASE.apply(call, visitor)returnsnullfor an operator carryingSqlKind.OTHER, and there is noconvertCase(...)to call instead. The caller's only options are to reconstruct theRexCallagainstSqlStdOperatorTable.CASEbefore delegating, or to copy the body.Copying is the part that will bite.
CAST's body carries an invariant documented atCallConverters.java:41-45: a nullability-only cast over a non-null literal is howExpressionRexConverterencodes a nullable Substrait literal, so it must fold back to a literal rather than anExpression.Cast. A downstream copy of that body is a fork of an internal encoding — when the encoding changes, every copy silently produces plans that no longer round-trip, and nothing in this repository can detect it.CASE's body likewise carries the odd-operand validation and the query-order requirement of theIfClauseloop.The precedent is already in the same file
EXECUTION_CONTEXT_VARIABLEhandles exactly this situation and its javadoc says so (CallConverters.java:273):"An operator with no dedicated
SqlKind" is precisely the dialect case. One converter in the set already matches on operator identity instead of kind; the rest cannot be asked to.Proposal
Separate the predicate from the conversion, so a dialect supplies only what differs:
CallConverters.cast(TypeConverter, Predicate<RexCall>),CallConverters.caseWhen(Predicate<RexCall>)— with the existing constants becoming the kind-based instances of those, orCallConvertercan invoke after its own matching.Either keeps one copy of the CAST literal-folding and CASE validation logic in this repository and reduces a dialect converter to its predicate.
Notes
Independent of #1166 — the transform decides where a converter sits, not whether the built-in conversion can be reached. Adjacent to but not covered by #1012, which is about the function-mapping layer (
Sigtables, the matcher, reverse lookup) rather than these non-function converters.