Type a nested field access as the field's type, not the whole row

[vinaykpud]

Description

Nested fields don't work in PPL right now when the query runs through the analytics engine. The engine hands Calcite a nested field as an array of rows (ARRAY<ROW<...>>), so something like parent.child turns into ITEM(parent, 'child') (where parent is the array and child is the field name).

The problem is that Calcite ignores the field name when it works out the type, so it types the result as the whole row instead of the child field. Any operation on it then breaks with:

Unsupported conversion for Relational Data type: ROW

So even a simple where parent.child > 4 fails.

This PR fixes the return type: when ITEM is used on an array of rows with a field name, we look the field up in the row and use its type. It's nullable because an empty array returns null. Regular array indexing (arr[0]) and map access (map['key']) are not affected.

Added a unit test for these cases.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit e3207f7)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Null Pointer Risk literal.getValueAs(String.class) can return null if the literal's value is SQL NULL. Passing null to component.getField(fieldName, ...) may cause a NullPointerException depending on the implementation. This occurs when a query uses ITEM(array_of_rows, NULL). String fieldName = literal.getValueAs(String.class); RelDataTypeField field = component.getField(fieldName, true, false);

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to e3207f7

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null check for field name Add null-safety check for fieldName before calling component.getField(). The literal.getValueAs(String.class) can return null, which could cause unexpected behavior when passed to getField(). core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [1383-1396] 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) { - ... + if (fieldName != null) { + RelDataTypeField field = component.getField(fieldName, true, false); + if (field != null) { + ... + } } } Suggestion importance[1-10]: 5 __ Why: While adding a null check for fieldName is a defensive programming practice, the existing code already handles the case where field is null (line 1389). If fieldName is null, component.getField() would likely return null, which is already handled. The suggestion improves robustness but doesn't address a critical bug.	Low

---

Previous suggestions

Suggestions up to commit 00b639e

Category	Suggestion	Impact
Possible issue	Add null check for field name Add null-safety check for fieldName before calling component.getField(). The literal.getValueAs(String.class) can return null, which could cause unexpected behavior when passed to getField(). core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [1382-1395] 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) { - ... + if (fieldName != null) { + RelDataTypeField field = component.getField(fieldName, true, false); + if (field != null) { + ... + } } } Suggestion importance[1-10]: 5 __ Why: While adding a null check for fieldName is a defensive programming practice, the existing code already handles the case where getField() returns null (line 1387). If fieldName is null, getField() would likely return null, and the code would fall through to the default behavior at line 1396. The suggestion improves robustness but doesn't address a critical bug.	Low

Suggestions up to commit f7bf594

Category	Suggestion	Impact
Possible issue	Add null check for field name Add null-safety check for fieldName before calling getField(). The getValueAs(String.class) method could potentially return null, which would cause a NullPointerException when passed to getField(). core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [1382-1395] 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) { - ... + if (fieldName != null) { + RelDataTypeField field = component.getField(fieldName, true, false); + if (field != null) { + ... + } } } Suggestion importance[1-10]: 5 __ Why: While adding a null check for fieldName is a defensive programming practice, RexLiteral.getValueAs(String.class) typically returns a non-null value for CHARACTER types in Calcite. The suggestion addresses a theoretical edge case but may not represent a realistic issue in practice. The impact is moderate as it improves robustness without being critical.	Low

Suggestions up to commit f43e49b

Category	Suggestion	Impact
Possible issue	Add null check for fieldName Add null-safety check for fieldName before calling getField(). The getValueAs(String.class) method could potentially return null, which would cause a NullPointerException when passed to getField(). core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java [1382-1395] 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) { - ... + if (fieldName != null) { + RelDataTypeField field = component.getField(fieldName, true, false); + if (field != null) { + ... + } } } Suggestion importance[1-10]: 5 __ Why: While adding a null check for fieldName is a defensive programming practice, getValueAs(String.class) on a RexLiteral with CHARACTER type family is unlikely to return null in practice. The suggestion improves robustness but addresses a low-probability edge case. The existing fallback to stock ITEM behavior when field is null already provides some safety.	Low

[github-actions]

Persistent review updated to latest commit f7bf594

[github-actions]

Persistent review updated to latest commit 00b639e

[ahkcs]

Have you tried a field that's two levels deep, like where events.detail.code = 5?

From what I can tell, the whole remaining path arrives here as a single name — "detail.code" — and we then look for a field with that exact name, which doesn't exist. So it falls through to the old behavior and fails the same way as before the fix.

parent.child works, but anything deeper doesn't seem to, can we have test covering that case.

[github-actions]

Persistent review updated to latest commit e3207f7

[vinaykpud]

Yeah, you're right. Confirmed it: where events.detail.code = 5 throws EQUAL ... got [STRUCT, INTEGER], and a flat_object map value like events.attributes.<key> breaks the same way. Only one level deep (events.name, events.code) actually works.

It's exactly what you said. QualifiedNameResolver.resolveFieldAccess joins the rest of the path into a single key, so we end up with ITEM(events, 'detail.code') instead of ITEM(ITEM(events, 'detail'), 'code'). There's no field literally named detail.code, so getField returns null, we fall back to the plain ITEM, and it stays typed as the whole row. Same failure as before the fix.

Let's keep this PR to single level. I'll handle the deeper case with tests in a follow-up.