Skip to content

Commit 5fc778f

Browse files
committed
Restrict mvappend widening to nullability-only mismatch
Calling Calcite's leastRestrictive widens mixed numerics like INT + DECIMAL to a single common numeric type (e.g. DECIMAL(11,1)). The Calcite engine then casts each operand to that type at codegen — Integer(1) becomes BigDecimal with scale 1, which renders as 0.1 (or 0 after JSON round-trip), breaking testMvappendWithIntAndDouble that expects mvappend(1, 2.5) to return [1, 2.5]. The original goal was just to bridge the nullability-tag mismatch that synthesizes an array's component as INTEGER NULLABLE versus a bare literal's INTEGER NOT NULL. Limit the widening to that case via equalSansNullability and keep the ANY fallback for genuinely different types — preserving the Calcite engine's heterogeneous-Object[] runtime semantics that pre-existing tests rely on. Signed-off-by: Kai Huang <huangkaics@gmail.com> Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent 5ab670b commit 5fc778f

1 file changed

Lines changed: 18 additions & 12 deletions

File tree

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

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.apache.calcite.sql.SqlOperatorBinding;
2121
import org.apache.calcite.sql.type.SqlReturnTypeInference;
2222
import org.apache.calcite.sql.type.SqlTypeName;
23+
import org.apache.calcite.sql.type.SqlTypeUtil;
2324
import org.opensearch.sql.expression.function.ImplementorUDF;
2425
import org.opensearch.sql.expression.function.UDFOperandMetadata;
2526

@@ -78,18 +79,23 @@ private static RelDataType updateMostGeneralType(
7879
if (current == null) {
7980
return candidate;
8081
}
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);
82+
if (current.equals(candidate)) {
83+
return current;
84+
}
85+
// Tolerate a nullability-only mismatch by widening to the nullable form. {@code Object.equals}
86+
// distinguishes INTEGER NULLABLE from INTEGER NOT NULL — e.g. {@code array(1, 2)} synthesizes
87+
// an INTEGER NULLABLE component while a bare literal {@code 3} is INTEGER NOT NULL — which
88+
// would otherwise fall into the {@code ANY} fallback. Calcite's {@code ANY} type isn't
89+
// substrait-serializable, so any analytics-engine query passing through {@code mvappend} with
90+
// mixed-nullability operands would fail substrait conversion with "Unable to convert the type
91+
// ANY". For genuinely different types (INT + DOUBLE, INT + VARCHAR, …) keep the {@code ANY}
92+
// fallback unchanged: the Calcite engine relies on {@code Object[]} runtime semantics to
93+
// preserve heterogeneous values as-is, and widening to a single numeric type would corrupt
94+
// values at codegen time (e.g. casting integer 1 to {@code DECIMAL(11,1)}).
95+
if (SqlTypeUtil.equalSansNullability(typeFactory, current, candidate)) {
96+
return typeFactory.createTypeWithNullability(current, true);
97+
}
98+
return typeFactory.createSqlType(SqlTypeName.ANY);
9399
}
94100

95101
public static class MVAppendImplementor implements NotNullImplementor {

0 commit comments

Comments
 (0)