Address the PR comments

Signed-off-by: Srikanth Padakanti <[email protected]>

Author: Srikanth Padakanti

core/src/main/java/org/opensearch/sql/ast/tree/MvExpand.java

@@ -34,11 +34,6 @@ public MvExpand attach(UnresolvedPlan child) {
     return this;
   }
 
-  @Nullable
-  public Integer getLimit() {
-    return limit;
-  }
-
   @Override
   public List<UnresolvedPlan> getChild() {
     return this.child == null ? ImmutableList.of() : ImmutableList.of(this.child);

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java

@@ -813,7 +813,12 @@ public RelNode visitPatterns(Patterns node, CalcitePlanContext context) {
                 .toList();
         context.relBuilder.aggregate(context.relBuilder.groupKey(groupByList), aggCall);
         buildExpandRelNode(
-            context.relBuilder.field(node.getAlias()), node.getAlias(), node.getAlias(), context);
+            context.relBuilder.field(node.getAlias()),
+            node.getAlias(),
+            node.getAlias(),
+            null,
+            context);
+
         flattenParsedPattern(
             node.getAlias(),
             context.relBuilder.field(node.getAlias()),
@@ -1556,16 +1561,11 @@ public RelNode visitMvExpand(MvExpand node, CalcitePlanContext context) {
     Field arrayField = node.getField();
     RexInputRef arrayFieldRex = (RexInputRef) rexVisitor.analyze(arrayField, context);
 
-    // buildMvExpandRelNode(arrayFieldRex, arrayField.getField().toString(), null, context);
-
     // pass the per-document limit into the builder so it can be applied inside the UNNEST inner
     // query
     buildMvExpandRelNode(
         arrayFieldRex, arrayField.getField().toString(), null, node.getLimit(), context);
 
-    //    if (node.getLimit() != null) {
-    //      context.relBuilder.limit(0, node.getLimit());
-    //    }
     return context.relBuilder.peek();
   }
 
@@ -2830,7 +2830,7 @@ public RelNode visitExpand(Expand expand, CalcitePlanContext context) {
     RexInputRef arrayFieldRex = (RexInputRef) rexVisitor.analyze(arrayField, context);
     String alias = expand.getAlias();
 
-    buildExpandRelNode(arrayFieldRex, arrayField.getField().toString(), alias, context);
+    buildExpandRelNode(arrayFieldRex, arrayField.getField().toString(), alias, null, context);
 
     return context.relBuilder.peek();
   }
@@ -3132,46 +3132,27 @@ private void buildUnnestForLeft(
   }
 
   private void buildExpandRelNode(
-      RexInputRef arrayFieldRex, String arrayFieldName, String alias, CalcitePlanContext context) {
-
-    // Capture left node and its schema BEFORE calling build()
-    RelNode leftNode = context.relBuilder.peek();
-    RelDataType leftRowType = leftNode.getRowType();
-
-    // Create correlation variable while left is still on the builder stack
-    Holder<RexCorrelVariable> correlVariable = Holder.empty();
-    context.relBuilder.variable(correlVariable::set);
-
-    // Create correlated field access while left is still on the builder stack
-    // (preserve original expand semantics: use the input RexInputRef index)
-    RexNode correlArrayFieldAccess =
-        context.relBuilder.field(
-            context.rexBuilder.makeCorrel(leftRowType, correlVariable.get().id),
-            arrayFieldRex.getIndex());
-
-    // Materialize leftBuilt (this pops the left from the relBuilder stack)
-    RelNode leftBuilt = context.relBuilder.build();
-
-    // Use unified helper to build right/uncollect + correlate + cleanup
-    buildUnnestForLeft(
-        leftBuilt,
-        leftRowType,
-        arrayFieldRex.getIndex(),
-        arrayFieldName,
-        alias,
-        correlVariable,
-        correlArrayFieldAccess,
-        null,
-        context);
-  }
-
-  private void buildMvExpandRelNode(
-      RexInputRef arrayFieldRex,
+      RexNode arrayFieldRexNode,
       String arrayFieldName,
       String alias,
       Integer mvExpandLimit,
       CalcitePlanContext context) {
 
+    // Convert incoming RexNode to RexInputRef when possible; otherwise resolve by field name.
+    RexInputRef arrayFieldRex;
+    if (arrayFieldRexNode instanceof RexInputRef) {
+      arrayFieldRex = (RexInputRef) arrayFieldRexNode;
+    } else {
+      RelDataType currentRowType = context.relBuilder.peek().getRowType();
+      RelDataTypeField fld = currentRowType.getField(arrayFieldName, false, false);
+      if (fld != null) {
+        arrayFieldRex = context.rexBuilder.makeInputRef(currentRowType, fld.getIndex());
+      } else {
+        throw new IllegalArgumentException(
+            "buildExpandRelNode: expected RexInputRef or resolvable field name: " + arrayFieldName);
+      }
+    }
+
     // Capture left node and its schema BEFORE calling build()
     RelNode leftNode = context.relBuilder.peek();
     RelDataType leftRowType = leftNode.getRowType();
@@ -3207,6 +3188,17 @@ private void buildMvExpandRelNode(
         context);
   }
 
+  private void buildMvExpandRelNode(
+      RexInputRef arrayFieldRex,
+      String arrayFieldName,
+      String alias,
+      Integer mvExpandLimit,
+      CalcitePlanContext context) {
+
+    // Delegate to the canonical expand implementation (pass the per-document limit through).
+    buildExpandRelNode(arrayFieldRex, arrayFieldName, alias, mvExpandLimit, context);
+  }
+
   /** Creates an optimized sed call using native Calcite functions */
   private RexNode createOptimizedSedCall(
       RexNode fieldRex, String sedExpression, CalcitePlanContext context) {

docs/user/ppl/cmd/mvexpand.rst

@@ -154,14 +154,19 @@ Output (example)::
     | error |
     +-------+
 
-Example 5: Large Arrays and Memory Limits
-----------------------------------------
-If an array is very large it can trigger engine/cluster resource limits (memory, circuit-breakers, or query execution limits). Note: this behavior is enforced by the underlying engine and cluster settings, not by a mvexpand-specific configuration.
+Example 5: Large Arrays and Memory / resource limits
+----------------------------------------------------
+If an array is very large it can trigger engine or cluster resource limits and the query can fail with an error. There is no mvexpand-specific configuration flag that controls resource usage; instead, limits are enforced by the engine and the cluster:
+
+- OpenSearch node-level protections (circuit breakers and JVM/heap safeguards) and request-size protections.
+- SQL/PPL execution limits (for example, query timeouts, request-size limits, and engine memory budgets) that apply to the query execution layer.
+
+Behavior of circuit breakers and which operators they protect can vary by release and configuration (some breakers primarily protect memory-heavy operations such as fielddata, aggregations, and certain scan implementations). Because of these distinctions, mvexpand should not be relied on to bypass cluster-level protections — use the command-level ``limit`` to bound per-document expansion and avoid hitting cluster limits.
 
 To avoid failures when expanding large arrays:
 - Use the `limit` parameter to restrict the number of expanded values per document (for example: `mvexpand field limit=1000`).
 - Filter or narrow the input before expanding (use `where` and `fields` to reduce rows and columns).
-- Tune cluster and SQL/PPL execution settings (circuit breakers, query size/timeouts, memory limits) appropriate for your deployment.
+- Tune cluster and SQL/PPL execution settings (circuit breakers, request/response size, timeouts, memory limits) appropriate for your deployment. If desired, we can add links to the exact OpenSearch circuit-breaker and SQL/PPL configuration docs for the targeted release.
 
 PPL query::