From dc1c66d14b38ad122312b99861fca20794701504 Mon Sep 17 00:00:00 2001
From: James Duong <james.duong@improving.com>
Date: Tue, 24 Sep 2024 16:13:56 -0700
Subject: [PATCH] PR feedback

- Add comments about method splitting
- Add overloads with default values
- Clarify condition about using a temporary writer
---
 .../calcite/linq4j/tree/Expressions.java       | 18 ++++++++++++++++++
 .../calcite/linq4j/tree/MethodDeclaration.java |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expressions.java b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expressions.java
index 88535507673..9a0585634e9 100644
--- a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expressions.java
+++ b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expressions.java
@@ -57,6 +57,16 @@ private Expressions() {}
    * Converts a list of expressions to Java source code, optionally emitting
    * extra type information in generics.
    */
+  public static String toString(List<? extends Node> expressions, String sep,
+      boolean generics) {
+    return toString(expressions, sep, generics, false);
+  }
+
+  /**
+   * Converts a list of expressions to Java source code, optionally emitting
+   * extra type information in generics. Optionally splits the generated method if
+   * the method is too large.
+   */
   public static String toString(List<? extends Node> expressions, String sep,
       boolean generics, boolean methodSplit) {
     final ExpressionWriter writer = new ExpressionWriter(generics, methodSplit);
@@ -70,6 +80,14 @@ public static String toString(List<? extends Node> expressions, String sep,
   /**
    * Converts an expression to Java source code.
    */
+  public static String toString(Node expression) {
+    return toString(expression, false);
+  }
+
+  /**
+   * Converts an expression to Java source code.
+   * Optionally splits the generated method if the method is too large.
+   */
   public static String toString(Node expression, boolean methodSplit) {
     return toString(Collections.singletonList(expression), "", true, methodSplit);
   }
diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/MethodDeclaration.java b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/MethodDeclaration.java
index 5410ad2fd6c..62537218961 100644
--- a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/MethodDeclaration.java
+++ b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/MethodDeclaration.java
@@ -60,6 +60,9 @@ public MethodDeclaration(int modifier, String name, Type resultType,
   }
 
   @Override public void accept(ExpressionWriter writer) {
+    // Conditionally serialize the method declaration directly to the supplied writer if method
+    // splitting is enabled or to a temporary writer that will generate code for the method that
+    // can be split before being serialized to the supplied writer.
     final ExpressionWriter writerForUnsplitMethod = writer.usesMethodSplitting()
         ? writer.duplicateState() : writer;