Skip to content

Commit a962f08

Browse files
committed
Use leastRestrictive for mvappend element-type widening
`MVAppendFunctionImpl.updateMostGeneralType` used strict {@code Object.equals} to compare each operand's component type against the running "most general" type, falling back to Calcite's {@code ANY} on any mismatch. That's too aggressive: {@code Object.equals} returns false for type pairs that differ only in nullability tag (e.g. {@code array(1, 2)} synthesizes INTEGER NULLABLE for its component while literal {@code 3} is INTEGER NOT NULL), and for straightforwardly-widenable numerics like INTEGER + DOUBLE. The PPL UDF result would then be {@code ARRAY<ANY>}. The Calcite engine's enumerable runtime tolerates {@code ANY} because {@code MVAppendImplementor.eval} processes elements through {@code Object} — the declared element type is unused at execution time. The analytics-engine route is stricter: substrait can't serialize {@code ANY}, so isthmus throws {@code UnsupportedOperationException: Unable to convert the type ANY} during the substrait conversion phase. Widen with {@link RelDataTypeFactory#leastRestrictive} — the same routine {@code SqlLibraryOperators.ARRAY} uses for its return-type inference. Falls back to ANY only when {@code leastRestrictive} returns null (genuinely incompatible operand types like INT + VARCHAR), preserving the original behavior on those queries. # Test plan * {@code :core:test --tests "*MVAppend*"} — passes (no existing test asserted on the {@code ANY} fallback). * Companion to opensearch-project/OpenSearch#21554 — unblocks 8+ tests in {@code CalciteMVAppendFunctionIT} force-routed through the analytics-engine path that previously failed with "Unable to convert the type ANY". Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent 109375d commit a962f08

1 file changed

Lines changed: 12 additions & 6 deletions

File tree

core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/MVAppendFunctionImpl.java

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,18 @@ private static RelDataType updateMostGeneralType(
7878
if (current == null) {
7979
return candidate;
8080
}
81-
82-
if (!current.equals(candidate)) {
83-
return typeFactory.createSqlType(SqlTypeName.ANY);
84-
} else {
85-
return current;
86-
}
81+
// Widen via Calcite's leastRestrictive (the same routine SqlLibraryOperators.ARRAY uses
82+
// for its return-type inference) instead of strict Object.equals. {@code Object.equals}
83+
// returns false for type pairs that only differ in nullability tag — e.g. {@code array(1, 2)}
84+
// synthesizes INTEGER NULLABLE for its component while a literal {@code 3} is INTEGER NOT
85+
// NULL — which then fell into the {@code ANY} fallback. Calcite's {@code ANY} type isn't
86+
// substrait-serializable, so any analytics-engine query passing through {@code mvappend}
87+
// with mixed-nullability operands or with widenable numerics (INT + DOUBLE) would fail
88+
// substrait conversion with "Unable to convert the type ANY". {@code leastRestrictive}
89+
// returns null when no common type exists (genuinely incompatible types like INT + VARCHAR);
90+
// fall back to ANY in that case to preserve the original behavior on those queries.
91+
RelDataType least = typeFactory.leastRestrictive(java.util.List.of(current, candidate));
92+
return least != null ? least : typeFactory.createSqlType(SqlTypeName.ANY);
8793
}
8894

8995
public static class MVAppendImplementor implements NotNullImplementor {

0 commit comments

Comments
 (0)