Merge pull request #1283 from katzyn/external

Clean up interaction between LocalResult and ResultExternal

h2/src/main/org/h2/mvstore/db/MVTempResult.java

@@ -186,9 +186,4 @@ public abstract class MVTempResult implements ResultExternal {
         tempFileDeleter.deleteFile(fileRef, closeable);
     }
 
-    @Override
-    public void done() {
-        // Do nothing
-    }
-
 }

h2/src/main/org/h2/result/LocalResult.java

@@ -42,7 +42,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
     private int offset;
     private int limit = -1;
     private ResultExternal external;
-    private int diskOffset;
     private boolean distinct;
     private boolean randomAccess;
     private boolean closed;
@@ -157,7 +156,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         copy.offset = 0;
         copy.limit = -1;
         copy.external = e2;
-        copy.diskOffset = this.diskOffset;
         return copy;
     }
 
@@ -231,11 +229,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         currentRow = null;
         if (external != null) {
             external.reset();
-            if (diskOffset > 0) {
-                for (int i = 0; i < diskOffset; i++) {
-                    external.next();
-                }
-            }
         }
     }
 
@@ -318,19 +311,19 @@ public class LocalResult implements ResultInterface, ResultTarget {
             } else {
                 rowCount = external.addRow(values);
             }
-            return;
-        }
+        } else {
             rows.add(values);
             rowCount++;
             if (rows.size() > maxMemoryRows) {
-            if (external == null) {
-                createExternalResult();
-            }
                 addRowsToDisk();
             }
         }
+    }
 
     private void addRowsToDisk() {
+        if (external == null) {
+            createExternalResult();
+        }
         rowCount = external.addRows(rows);
         rows.clear();
     }
@@ -344,52 +337,76 @@ public class LocalResult implements ResultInterface, ResultTarget {
      * This method is called after all rows have been added.
      */
     public void done() {
+        if (external != null) {
+            addRowsToDisk();
+        } else {
             if (distinct) {
-            if (distinctRows != null) {
                 rows = distinctRows.values();
+            }
+            if (sort != null) {
+                if (offset > 0 || limit > 0) {
+                    sort.sort(rows, offset, limit < 0 ? rows.size() : limit);
                 } else {
-                if (external != null && sort != null) {
-                    // external sort
-                    ResultExternal temp = external;
-                    external = null;
-                    temp.reset();
-                    rows = Utils.newSmallArrayList();
-                    // TODO use offset directly if possible
-                    while (true) {
-                        Value[] list = temp.next();
-                        if (list == null) {
-                            break;
+                    sort.sort(rows);
                 }
-                        if (external == null) {
-                            createExternalResult();
             }
-                        rows.add(list);
-                        if (rows.size() > maxMemoryRows) {
-                            rowCount = external.addRows(rows);
+        }
+        applyOffsetAndLimit();
+        reset();
+    }
+
+    private void applyOffsetAndLimit() {
+        int offset = Math.max(this.offset, 0);
+        int limit = this.limit;
+        if (offset == 0 && limit < 0 || rowCount == 0) {
+            return;
+        }
+        boolean clearAll = offset >= rowCount || limit == 0;
+        if (!clearAll) {
+            int remaining = rowCount - offset;
+            limit = limit < 0 ? remaining : Math.min(remaining, limit);
+            if (offset == 0 && remaining <= limit) {
+                return;
+            }
+        } else {
+            limit = 0;
+        }
+        distinctRows = null;
+        rowCount = limit;
+        if (external == null) {
+            if (clearAll) {
                 rows.clear();
+                return;
             }
+            // avoid copying the whole array for each row
+            rows = new ArrayList<>(rows.subList(offset, offset + limit));
+        } else {
+            if (clearAll) {
+                external.close();
+                external = null;
+                return;
             }
-                    temp.close();
-                    // the remaining data in rows is written in the following
-                    // lines
+            trimExternal(offset, limit);
         }
     }
+
+    private void trimExternal(int offset, int limit) {
+        ResultExternal temp = external;
+        external = null;
+        temp.reset();
+        while (--offset >= 0) {
+            temp.next();
         }
-        if (external != null) {
+        while (--limit >= 0) {
+            rows.add(temp.next());
+            if (rows.size() > maxMemoryRows) {
                 addRowsToDisk();
-            external.done();
-        } else {
-            if (sort != null) {
-                if (offset > 0 || limit > 0) {
-                    sort.sort(rows, offset, limit < 0 ? rows.size() : limit);
-                } else {
-                    sort.sort(rows);
             }
         }
+        if (external != null) {
+            addRowsToDisk();
         }
-        applyOffset();
-        applyLimit();
-        reset();
+        temp.close();
     }
 
     @Override
@@ -411,24 +428,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         this.limit = limit;
     }
 
-    private void applyLimit() {
-        if (limit < 0) {
-            return;
-        }
-        if (external == null) {
-            if (rows.size() > limit) {
-                rows = new ArrayList<>(rows.subList(0, limit));
-                rowCount = limit;
-                distinctRows = null;
-            }
-        } else {
-            if (limit < rowCount) {
-                rowCount = limit;
-                distinctRows = null;
-            }
-        }
-    }
-
     @Override
     public boolean needToClose() {
         return external != null;
@@ -502,31 +501,6 @@ public class LocalResult implements ResultInterface, ResultTarget {
         this.offset = offset;
     }
 
-    private void applyOffset() {
-        if (offset <= 0) {
-            return;
-        }
-        if (external == null) {
-            if (offset >= rows.size()) {
-                rows.clear();
-                rowCount = 0;
-            } else {
-                // avoid copying the whole array for each row
-                int remove = Math.min(offset, rows.size());
-                rows = new ArrayList<>(rows.subList(remove, rows.size()));
-                rowCount -= remove;
-            }
-        } else {
-            if (offset >= rowCount) {
-                rowCount = 0;
-            } else {
-                diskOffset = offset;
-                rowCount -= offset;
-            }
-        }
-        distinctRows = null;
-    }
-
     @Override
     public String toString() {
         return super.toString() + " columns: " + visibleColumnCount +

h2/src/main/org/h2/result/ResultExternal.java

@@ -42,11 +42,6 @@ public interface ResultExternal {
      */
     int addRows(ArrayList<Value[]> rows);
 
-    /**
-     * This method is called after all rows have been added.
-     */
-    void done();
-
     /**
      * Close this object and delete the temporary file.
      */

h2/src/main/org/h2/result/ResultTempTable.java

@@ -309,11 +309,6 @@ public class ResultTempTable implements ResultExternal {
         }
     }
 
-    @Override
-    public void done() {
-        // nothing to do
-    }
-
     @Override
     public Value[] next() {
         if (resultCursor == null) {