Skip to content

Commit c42fb7c

Browse files
committed
fix(opensearch): revert arity-aware toOp; restore spath/JSON_EXTRACT doctest
The arity filter added to ExtendedRelJson.toOp in the previous commit broke SAFE_CAST → JSON_EXTRACT deserialization (used by `spath` lowering): the PPL JSON_EXTRACT UDF, registered as an anonymous UserDefinedFunctionBuilder subclass, doesn't expose a meaningful getOperandCountRange(), so my filter fell through to the firstKindMatch path and skipped the AvaticaUtils.instantiatePlugin "class" path that previously resolved the UDF. spath.md doctest started returning RuntimeException on `source=structured | spath input=doc_n n | eval n=cast(n as int) | stats sum(n)`. The RexStandardizer collapse (4-arg `REGEXP_REPLACE_PG_4(..., 'g')` → 3-arg `REGEXP_REPLACE_3`) already fixes the patterns.md doctest at the source side — by the time pushdown serialization runs, no 4-arg call exists for toOp to disambiguate. The arity filter was defensive only and no longer carries its weight; revert toOp to the original first-kind-match behavior, plus a spotless re-flow that came in with the same change. Verified locally on a fresh cluster: - spath.md doctest query → returns sum(n)=6 (was 500). - patterns.md doctest query → returns fully-tokenized aggregation rows. - CalcitePPLDashboardPatternsIT → 1/1 PASS. - CalcitePPLPatternsIT → 10/15 PASS (same baseline; same five known-pending BRAIN failures tracked separately). Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent 8720ca7 commit c42fb7c

2 files changed

Lines changed: 5 additions & 33 deletions

File tree

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/ExtendedRelJson.java

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,7 @@ RexNode toRex(RelInput relInput, @PolyNull Object o) {
460460
distinct,
461461
false);
462462
} else {
463-
final SqlOperator operator =
464-
requireNonNull(toOp(opMap, rexOperands.size()), "operator");
463+
final SqlOperator operator = requireNonNull(toOp(opMap), "operator");
465464
final RelDataType type;
466465
if (jsonType != null) {
467466
type = toType(typeFactory, jsonType);
@@ -554,22 +553,6 @@ private static <T extends Object> T get(Map<String, ? extends @Nullable Object>
554553

555554
// Copied from RelJson for the usage of custom operatorTable
556555
@Nullable SqlOperator toOp(Map<String, ? extends @Nullable Object> map) {
557-
return toOp(map, -1);
558-
}
559-
560-
/**
561-
* Look up an operator matching the {@code map}'s {@code name} / {@code kind} / {@code syntax}.
562-
*
563-
* <p>When {@code operandCount >= 0}, prefer an overload whose declared operand-count range
564-
* accepts {@code operandCount} — Calcite registers multiple operators under the same SQL name
565-
* with different arities (e.g. {@code REGEXP_REPLACE} maps to {@code REGEXP_REPLACE_3},
566-
* {@code REGEXP_REPLACE_PG_4}, and {@code REGEXP_REPLACE_5} in PostgreSQL library). Picking by
567-
* name + kind alone returns whichever overload is listed first, so a 4-arg call would bind to
568-
* the 3-arg operator and fail at runtime with {@code IllegalArgumentException: no matching
569-
* method} during {@code RexToLixTranslator} code-gen. Fall back to the first kind match when
570-
* no overload's arity range fits, preserving prior behavior for callers that pass {@code -1}.
571-
*/
572-
@Nullable SqlOperator toOp(Map<String, ? extends @Nullable Object> map, int operandCount) {
573556
// in case different operator has the same kind, check with both name and kind.
574557
String name = get(map, "name");
575558
String kind = get(map, "kind");
@@ -583,21 +566,11 @@ private static <T extends Object> T get(Map<String, ? extends @Nullable Object>
583566
sqlSyntax,
584567
operators,
585568
SqlNameMatchers.liberal());
586-
SqlOperator firstKindMatch = null;
587569
for (SqlOperator operator : operators) {
588-
if (operator.kind != sqlKind) {
589-
continue;
590-
}
591-
if (firstKindMatch == null) {
592-
firstKindMatch = operator;
593-
}
594-
if (operandCount < 0 || operator.getOperandCountRange().isValidCount(operandCount)) {
570+
if (operator.kind == sqlKind) {
595571
return operator;
596572
}
597573
}
598-
if (firstKindMatch != null) {
599-
return firstKindMatch;
600-
}
601574
String class_ = (String) map.get("class");
602575
if (class_ != null) {
603576
return AvaticaUtils.instantiatePlugin(SqlOperator.class, class_);

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,10 @@ public RexNode visitCall(final RexCall call, ScriptParameterHelper helper) {
8888
&& call.operands.size() == 4
8989
&& call.operands.get(3) instanceof RexLiteral flagsLit
9090
&& "g".equals(flagsLit.getValueAs(String.class))) {
91-
return helper.rexBuilder
91+
return helper
92+
.rexBuilder
9293
.makeCall(
93-
call.getType(),
94-
SqlLibraryOperators.REGEXP_REPLACE_3,
95-
call.operands.subList(0, 3))
94+
call.getType(), SqlLibraryOperators.REGEXP_REPLACE_3, call.operands.subList(0, 3))
9695
.accept(this, helper);
9796
}
9897
// Some functions only support limited numeric type. Keep conservative here.

0 commit comments

Comments
 (0)