drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From meh...@apache.org
Subject [1/5] drill git commit: DRILL-3285: Part 5--Split old hacky next() into separate methods.
Date Tue, 23 Jun 2015 19:00:05 GMT
Repository: drill
Updated Branches:
  refs/heads/master 9c125b0d9 -> 711992f22


DRILL-3285: Part 5--Split old hacky next() into separate methods.

Split the original public next() method (which was hacked to handle an extra,
initial call to read the schema batch) into:
- new loadInitialSchema() (for handling the call for the schema)
- modified next() (for handling normal calls from ResultSet.next())
- new private nextRowInternally() (for common code)

Pulled invariant afterFirstBatch up out of bogus-batch loop.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/711992f2
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/711992f2
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/711992f2

Branch: refs/heads/master
Commit: 711992f22ae6d6dfc43bdb4c01bf8f921d175b38
Parents: 22232d4
Author: dbarclay <dbarclay@maprtech.com>
Authored: Wed Jun 10 13:49:47 2015 -0700
Committer: Mehant Baid <mehantr@gmail.com>
Committed: Mon Jun 22 13:05:04 2015 -0700

----------------------------------------------------------------------
 .../org/apache/drill/jdbc/impl/DrillCursor.java | 124 +++++++++++++------
 .../drill/jdbc/impl/DrillResultSetImpl.java     |   2 +-
 2 files changed, 84 insertions(+), 42 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/711992f2/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillCursor.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillCursor.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillCursor.java
index cdf030b..5ae7509 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillCursor.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillCursor.java
@@ -56,8 +56,7 @@ class DrillCursor implements Cursor {
   /** ... corresponds to current schema. */
   private DrillColumnMetaDataList columnMetaDataList;
 
-  /** Whether we're past the special first call to this.next() from
-   * DrillResultSetImpl.execute(). */
+  /** Whether loadInitialSchema() has been called. */
   private boolean initialSchemaLoaded = false;
 
   /** Whether after first batch.  (Re skipping spurious empty batches.) */
@@ -65,7 +64,8 @@ class DrillCursor implements Cursor {
 
   /**
    * Whether the next call to this.next() should just return {@code true} rather
-   * than trying to actually advance to the next record.
+   * than calling nextRowInternally() to try to advance to the next
+   * record.
    * <p>
    *   Currently, can be true only for first call to next().
    * </p>
@@ -120,33 +120,17 @@ class DrillCursor implements Cursor {
   }
 
   /**
-   * Advances this cursor to the next row, if any, or to after the sequence of
-   * rows if no next row.  However, the first call does not advance to the first
-   * row, only reading schema information.
+   * ...
    * <p>
-   *   Is to be called (once) from {@link DrillResultSetImpl#execute()}, and
-   *   then from {@link AvaticaResultSet#next()}.
+   *   Is to be called (once) from {@link #loadInitialSchema} for
+   *   {@link DrillResultSetImpl#execute()}, and then (repeatedly) from
+   *   {@link #next()} for {@link AvaticaResultSet#next()}.
    * </p>
    *
    * @return  whether cursor is positioned at a row (false when after end of
    *   results)
    */
-  @Override
-  public boolean next() throws SQLException {
-    if (!initialSchemaLoaded) {
-      initialSchemaLoaded = true;
-      returnTrueForNextCallToNext = true;
-    } else if (returnTrueForNextCallToNext && !afterLastRow) {
-      // We have a deferred "not after end" to report--reset and report that.
-      returnTrueForNextCallToNext = false;
-      return true;
-    }
-
-    if (afterLastRow) {
-      // We're already after end of rows/records--just report that after end.
-      return false;
-    }
-
+  private boolean nextRowInternally() throws SQLException {
     if (currentRecordNumber + 1 < currentBatchHolder.getRecordCount()) {
       // Have next row in current batch--just advance index and report "at a row."
       currentRecordNumber++;
@@ -161,28 +145,29 @@ class DrillCursor implements Cursor {
         // (Apparently:)  Skip any spurious empty batches (batches that have
         // zero rows and/or null data, other than the first batch (which carries
         // the (initial) schema but no rows)).
-        while ( qrb != null
-                && ( qrb.getHeader().getRowCount() == 0
-                     || qrb.getData() == null )
-                && afterFirstBatch ) {
-          // Empty message--dispose of and try to get another.
-          logger.warn( "Spurious batch read: {}", qrb );
+        if ( afterFirstBatch ) {
+          while ( qrb != null
+                  && ( qrb.getHeader().getRowCount() == 0
+                      || qrb.getData() == null ) ) {
+            // Empty message--dispose of and try to get another.
+            logger.warn( "Spurious batch read: {}", qrb );
 
-          qrb.release();
+            qrb.release();
 
-          qrb = resultsListener.getNext();
+            qrb = resultsListener.getNext();
 
-          // NOTE:  It is unclear why this check does not check getRowCount()
-          // as the loop condition above does.
-          if ( qrb != null && qrb.getData() == null ) {
-            // Got another batch with null data--dispose of and report "no more
-            // rows".
+            // NOTE:  It is unclear why this check does not check getRowCount()
+            // as the loop condition above does.
+            if ( qrb != null && qrb.getData() == null ) {
+              // Got another batch with null data--dispose of and report "no more
+              // rows".
 
-            qrb.release();
+              qrb.release();
 
-            // NOTE:  It is unclear why this returns false but doesn't set
-            // afterLastRow (as we do when we normally return false).
-            return false;
+              // NOTE:  It is unclear why this returns false but doesn't set
+              // afterLastRow (as we do when we normally return false).
+              return false;
+            }
           }
         }
 
@@ -246,6 +231,63 @@ class DrillCursor implements Cursor {
     }
   }
 
+  /**
+   * Advances to first batch to load schema data into result set metadata.
+   * <p>
+   *   To be called once from {@link DrillResultSetImpl#execute()} before
+   *   {@link next()} is called from {@link AvaticaResultSet#next()}.
+   * <p>
+   */
+  void loadInitialSchema() throws SQLException {
+    if ( initialSchemaLoaded ) {
+      throw new IllegalStateException(
+          "loadInitialSchema() called a second time" );
+    }
+    assert ! afterLastRow : "afterLastRow already true in loadInitialSchema()";
+    assert ! afterFirstBatch : "afterLastRow already true in loadInitialSchema()";
+    assert -1 == currentRecordNumber
+        : "currentRecordNumber not -1 (is " + currentRecordNumber
+          + ") in loadInitialSchema()";
+    assert 0 == currentBatchHolder.getRecordCount()
+        : "currentBatchHolder.getRecordCount() not 0 (is "
+          + currentBatchHolder.getRecordCount() + " in loadInitialSchema()";
+
+    returnTrueForNextCallToNext = true;
+
+    nextRowInternally();
+
+    initialSchemaLoaded = true;
+  }
+
+  /**
+   * Advances this cursor to the next row, if any, or to after the sequence of
+   * rows if no next row.
+   *
+   * @return  whether cursor is positioned at a row (false when after end of
+   *   results)
+   */
+  @Override
+  public boolean next() throws SQLException {
+    if ( ! initialSchemaLoaded ) {
+      throw new IllegalStateException(
+          "next() called but loadInitialSchema() was not called" );
+    }
+    assert afterFirstBatch : "afterFirstBatch still false in next()";
+
+    if ( afterLastRow ) {
+      // We're already after end of rows/records--just report that after end.
+      return false;
+    }
+    else if ( returnTrueForNextCallToNext ) {
+      // We have a deferred "not after end" to report--reset and report that.
+      returnTrueForNextCallToNext = false;
+      return true;
+    }
+    else {
+      return nextRowInternally();
+    }
+  }
+
   @Override
   public void close() {
     // currentBatchHolder is owned by resultSet and cleaned up by

http://git-wip-us.apache.org/repos/asf/drill/blob/711992f2/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
index c682d97..31593bf 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
@@ -184,7 +184,7 @@ class DrillResultSetImpl extends AvaticaResultSet implements DrillResultSet
{
 
     // Read first (schema-only) batch to initialize result-set metadata from
     // (initial) schema before Statement.execute...(...) returns result set:
-    cursor.next();
+    cursor.loadInitialSchema();
 
     return this;
   }


Mime
View raw message