diff --git a/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java b/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java index f02db636785..77e24642fcb 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java @@ -291,6 +291,7 @@ import java.util.stream.Stream; import javax.annotation.Nullable; import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.RelDataTypeField; import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexCall; import org.apache.calcite.rex.RexLambda; @@ -1365,6 +1366,37 @@ void populate() { OperandTypes.family(SqlTypeFamily.ARRAY, SqlTypeFamily.INTEGER) .or(OperandTypes.family(SqlTypeFamily.MAP, SqlTypeFamily.ANY)), false)); + // A `nested` mapping is exposed as ARRAY>, so `events.name` becomes + // ITEM(, 'name'). By default, Calcite types this as the whole ROW because it + // never looks at the field name — so a later `events.count > 4` fails with + // "Unsupported conversion for Relational Data type: ROW". + // Fix: look 'name' up in the ROW and type the result as that field. Must be registered before + // the (IGNORE, CHARACTER) catch-all in the next registration below — that fallback would + // otherwise match ITEM(, 'name') first and re-apply the stock whole-ROW + // typing. + register( + INTERNAL_ITEM, + (FunctionImp2) + (builder, array, key) -> { + RelDataType arrayType = array.getType(); + RelDataType component = arrayType.getComponentType(); + if (component != null + && component.isStruct() + && key instanceof RexLiteral literal + && SqlTypeFamily.CHARACTER.contains(literal.getType())) { + String fieldName = literal.getValueAs(String.class); + RelDataTypeField field = component.getField(fieldName, true, false); + if (field != null) { + // Nullable: an empty array yields NULL, independent of field nullability. + RelDataType fieldType = + builder.getTypeFactory().createTypeWithNullability(field.getType(), true); + return builder.makeCall( + fieldType, SqlStdOperatorTable.ITEM, List.of(array, key)); + } + } + return builder.makeCall(SqlStdOperatorTable.ITEM, array, key); + }, + PPLTypeChecker.family(SqlTypeFamily.ARRAY, SqlTypeFamily.CHARACTER)); registerOperator( INTERNAL_ITEM, SqlStdOperatorTable.ITEM, diff --git a/core/src/test/java/org/opensearch/sql/expression/function/PPLFuncImpTableNestedItemTest.java b/core/src/test/java/org/opensearch/sql/expression/function/PPLFuncImpTableNestedItemTest.java new file mode 100644 index 00000000000..374a5e6f45a --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/function/PPLFuncImpTableNestedItemTest.java @@ -0,0 +1,108 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.expression.function; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.TYPE_FACTORY; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.INTERNAL_ITEM; + +import java.math.BigDecimal; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rex.RexBuilder; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.sql.type.SqlTypeName; +import org.junit.jupiter.api.Test; + +/** + * Return-type behavior of {@code ITEM} (the internal {@code item} builtin behind {@code + * events.name}, {@code arr[0]}, {@code map['k']}) — specifically the nested-field case where a + * {@code nested} mapping is exposed as {@code ARRAY>}. Stock Calcite types {@code + * ITEM(ARRAY, 'field')} as the whole {@code ROW}; the override resolves it to the named + * field's type instead. Array-index and map-key access must be left untouched. + */ +public class PPLFuncImpTableNestedItemTest { + + private final RexBuilder builder = new RexBuilder(TYPE_FACTORY); + + /** ARRAY<ROW<name:VARCHAR, count:INTEGER>> — how a nested mapping is exposed. */ + private RelDataType structArray() { + RelDataType row = + TYPE_FACTORY + .builder() + .add("name", TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR)) + .add("count", TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER)) + .build(); + return TYPE_FACTORY.createArrayType(row, -1); + } + + private RexNode item(RelDataType arrayType, RexNode key) { + RexNode arrayRef = builder.makeInputRef(arrayType, 0); + return PPLFuncImpTable.INSTANCE.resolve(builder, INTERNAL_ITEM, arrayRef, key); + } + + @Test + public void keywordLeafIsTypedAsTheFieldNotTheRow() { + RexNode result = item(structArray(), builder.makeLiteral("name")); + assertEquals(SqlTypeName.VARCHAR, result.getType().getSqlTypeName()); + assertFalse(result.getType().isStruct(), "must be the leaf field, not the whole ROW"); + assertTrue(result.getType().isNullable(), "an empty array yields NULL"); + } + + @Test + public void numericLeafIsTypedAsTheField() { + RexNode result = item(structArray(), builder.makeLiteral("count")); + assertEquals(SqlTypeName.INTEGER, result.getType().getSqlTypeName()); + assertFalse(result.getType().isStruct()); + } + + @Test + public void unknownFieldFallsBackToStockRowTyping() { + // No such field in the ROW: the override bails and defers to stock Calcite ITEM, which types + // the + // call as the array's component (the whole ROW) — behavior we deliberately do not change. + RexNode result = item(structArray(), builder.makeLiteral("missing")); + assertTrue(result.getType().isStruct(), "unknown field falls back to the whole ROW"); + } + + @Test + public void arrayIndexAccessIsUnaffected() { + // ITEM(ARRAY, 1) is (ARRAY, INTEGER) — not (ARRAY, CHARACTER) — so it never enters the + // override and keeps returning the element type. + RelDataType intArray = + TYPE_FACTORY.createArrayType(TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER), -1); + RexNode idx = + builder.makeExactLiteral(BigDecimal.ONE, TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER)); + RexNode result = item(intArray, idx); + assertEquals(SqlTypeName.INTEGER, result.getType().getSqlTypeName()); + } + + @Test + public void mapKeyAccessIsUnaffected() { + // ITEM(MAP, 'k') is (MAP, *) — not (ARRAY, *) — so the override's ARRAY guard + // skips it and it keeps returning the map value type. + RelDataType mapType = + TYPE_FACTORY.createMapType( + TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), + TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER)); + RexNode mapRef = builder.makeInputRef(mapType, 0); + RexNode result = + PPLFuncImpTable.INSTANCE.resolve(builder, INTERNAL_ITEM, mapRef, builder.makeLiteral("k")); + assertEquals(SqlTypeName.INTEGER, result.getType().getSqlTypeName()); + } + + @Test + public void stringKeyOnNonStructArrayFallsThrough() { + // (ARRAY, CHARACTER) matches the override's signature, but the component is not a ROW — the + // isStruct guard fails, so it defers to stock ITEM (no field lookup, type is the element type). + RelDataType stringArray = + TYPE_FACTORY.createArrayType(TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), -1); + RexNode result = item(stringArray, builder.makeLiteral("x")); + assertEquals(SqlTypeName.VARCHAR, result.getType().getSqlTypeName()); + assertFalse(result.getType().isStruct()); + } +}