fix: allow LIST/VALUES aggregates to accept DATE/TIMESTAMP/IP/BINARY field types

[waterWang]

Description

This PR fixes a type-matching bug in the PPL aggregate function validation that caused LIST and VALUES aggregate functions to reject DATE, TIMESTAMP, IP, and BINARY field types, even though these types were explicitly listed in the allowed signatures.

Root Cause

The typesMatch() method in PPLTypeChecker.java was returning false when one operand was a UDT (via AbstractExprRelDataType) and the other was a plain RelDataType. Database-sourced fields (e.g., TIMESTAMP from a table) arrive as plain RelDataType, but the allowed SCALAR_TYPES list in PPLOperandTypes.java uses UDT variants for temporal/special types (e.g., TIMESTAMP_UDT, DATE_UDT, IP_UDT, BINARY_UDT).

This caused the validation to fail with:

Aggregation function LIST expects field type {[BYTE]|[SHORT]|[INTEGER]|[LONG]|[FLOAT]|[DOUBLE]|[STRING]|[BOOLEAN]|[DATE]|[TIME]|[TIMESTAMP]|[IP]|[BINARY]}, but got [TIMESTAMP]

Fix

Changed typesMatch() to fall back to SqlTypeName comparison when one operand is a UDT and the other is a plain RelDataType, instead of returning false.

Related Issues

Fixes opensearch-project/OpenSearch#22581

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Incomplete Type Match The fix assumes that SqlTypeName comparison is sufficient when one operand is a UDT and the other is plain RelDataType. However, if both expected and actual are plain RelDataType instances but represent semantically different types that happen to share the same SqlTypeName, this could produce false positives. The original code explicitly rejected mixed UDT/plain comparisons, suggesting there may be cases where SqlTypeName alone is insufficient to determine type compatibility. Consider whether additional validation is needed to ensure the plain RelDataType truly corresponds to the UDT's semantic type. // Fall back to SqlTypeName comparison when one operand is a UDT and the other is a plain // RelDataType. This covers database-sourced fields (e.g. TIMESTAMP) that arrive as plain // RelDataType against allowed signatures using UDT variants (e.g. TIMESTAMP_UDT). return expected.getSqlTypeName() == actual.getSqlTypeName();

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null safety check Add null safety check before calling getSqlTypeName() on both expected and actual parameters. If either parameter is null, the method will throw a NullPointerException when attempting to call getSqlTypeName(). core/src/main/java/org/opensearch/sql/expression/function/PPLTypeChecker.java [566-569] // Fall back to SqlTypeName comparison when one operand is a UDT and the other is a plain // RelDataType. This covers database-sourced fields (e.g. TIMESTAMP) that arrive as plain // RelDataType against allowed signatures using UDT variants (e.g. TIMESTAMP_UDT). +if (expected == null || actual == null) { + return false; +} return expected.getSqlTypeName() == actual.getSqlTypeName(); Suggestion importance[1-10]: 5 __ Why: While adding null checks is generally good practice, the suggestion assumes expected and actual can be null without evidence from the PR context. The method signature and existing code don't indicate null handling is needed, and the removed code didn't check for nulls either. This is a defensive programming suggestion that may or may not be necessary depending on the broader codebase contract.	Low

[dai-chen]

Please add IT to verify.