rohangarg commented on code in PR #12909:
URL: https://github.com/apache/druid/pull/12909#discussion_r949531463


##########
sql/src/main/java/org/apache/druid/sql/DirectStatement.java:
##########
@@ -62,9 +67,91 @@ public class DirectStatement extends AbstractStatement 
implements Cancelable
 {
   private static final Logger log = new Logger(DirectStatement.class);
 
+  /**
+   * Represents the execution plan for a query with the ability to run
+   * that plan (once).
+   */
+  public class ResultSet implements Cancelable
+  {
+    private final PlannerResult plannerResult;
+
+    public ResultSet(PlannerResult plannerResult)
+    {
+      this.plannerResult = plannerResult;
+    }
+
+    public SqlQueryPlus query()
+    {
+      return queryPlus;
+    }
+
+    /**
+     * Convenience method for the split plan/run case to ensure that the 
statement
+     * can, in fact, be run.
+     */
+    public boolean runnable()
+    {
+      return plannerResult != null && plannerResult.runnable();
+    }
+
+    /**
+     * Do the actual execute step which allows subclasses to wrap the sequence,
+     * as is sometimes needed for testing.
+     */
+    public Sequence<Object[]> run()
+    {
+      try {
+        // Check cancellation. Required for SqlResourceTest to work.
+        transition(State.RAN);
+        return plannerResult.run();
+      }
+      catch (RuntimeException e) {
+        reporter.failed(e);
+        throw e;
+      }
+    }
+
+    public SqlRowTransformer createRowTransformer()
+    {
+      return new SqlRowTransformer(plannerContext.getTimeZone(), 
plannerResult.rowType());
+    }
+
+    public SqlExecutionReporter reporter()
+    {
+      return reporter;
+    }
+
+    @Override
+    public Set<ResourceAction> resources()
+    {
+      return DirectStatement.this.resources();
+    }
+
+    @Override
+    public void cancel()
+    {
+      DirectStatement.this.cancel();
+    }
+
+    public void close()
+    {
+      DirectStatement.this.close();
+    }
+  }
+
+  private enum State
+  {
+    START,
+    PREPARED,
+    RAN,

Review Comment:
   nit : s/`RAN`/`EXECUTED`



##########
sql/src/main/java/org/apache/druid/sql/DirectStatement.java:
##########
@@ -62,9 +67,91 @@ public class DirectStatement extends AbstractStatement 
implements Cancelable
 {
   private static final Logger log = new Logger(DirectStatement.class);
 
+  /**
+   * Represents the execution plan for a query with the ability to run
+   * that plan (once).
+   */
+  public class ResultSet implements Cancelable
+  {
+    private final PlannerResult plannerResult;
+
+    public ResultSet(PlannerResult plannerResult)
+    {
+      this.plannerResult = plannerResult;
+    }
+
+    public SqlQueryPlus query()
+    {
+      return queryPlus;
+    }
+
+    /**
+     * Convenience method for the split plan/run case to ensure that the 
statement
+     * can, in fact, be run.
+     */
+    public boolean runnable()
+    {
+      return plannerResult != null && plannerResult.runnable();
+    }
+
+    /**
+     * Do the actual execute step which allows subclasses to wrap the sequence,
+     * as is sometimes needed for testing.
+     */
+    public Sequence<Object[]> run()
+    {
+      try {
+        // Check cancellation. Required for SqlResourceTest to work.
+        transition(State.RAN);
+        return plannerResult.run();
+      }
+      catch (RuntimeException e) {
+        reporter.failed(e);
+        throw e;
+      }
+    }
+
+    public SqlRowTransformer createRowTransformer()
+    {
+      return new SqlRowTransformer(plannerContext.getTimeZone(), 
plannerResult.rowType());
+    }
+
+    public SqlExecutionReporter reporter()
+    {
+      return reporter;
+    }
+
+    @Override
+    public Set<ResourceAction> resources()
+    {
+      return DirectStatement.this.resources();
+    }
+
+    @Override
+    public void cancel()
+    {
+      DirectStatement.this.cancel();
+    }
+
+    public void close()
+    {
+      DirectStatement.this.close();
+    }
+  }
+
+  private enum State
+  {
+    START,

Review Comment:
   nit : s/`START`/`INIT` 



##########
sql/src/main/java/org/apache/druid/sql/DirectStatement.java:
##########
@@ -62,8 +65,81 @@ public class DirectStatement extends AbstractStatement 
implements Cancelable
 {
   private static final Logger log = new Logger(DirectStatement.class);
 
+  /**
+   * Represents the execution plan for a query with the ability to run
+   * that plan (once).
+   */
+  public class ResultSet implements Cancelable

Review Comment:
   Thanks a lot for explanation! Yes, I agree that the semblence to JDBC 
concept makes sense and I was also thinking in the same line. But I was 
thinking that maybe then the `run` or `execute` method should be a part of the 
statement instead of the resultset. Maybe, the result-set could start with a 
`Yielder` object over the result sequence.
   
   But agree that all that could be done after more thought as a follow up. And 
completely agree with the concept of breaking Statement into Statement and 
ResultRet :+1:



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to