From 278316b82e336ce9313811cd7da87bf83d6c2401 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Tue, 9 Jun 2015 18:16:37 -0700 Subject: [PATCH] Address comments from review and simplify code. --- .../accessmethod/SQLiteAccessMethod.java | 2 +- .../SQLiteTupleBatchIterator.java | 18 +++------- .../coordinator/catalog/MasterCatalog.java | 6 ++-- .../myria/operator/CatalogQueryScan.java | 21 +++-------- .../escience/myria/operator/DbQueryScan.java | 35 +++++-------------- .../myria/operator/CatalogScanTest.java | 4 +-- 6 files changed, 22 insertions(+), 64 deletions(-) diff --git a/src/edu/washington/escience/myria/accessmethod/SQLiteAccessMethod.java b/src/edu/washington/escience/myria/accessmethod/SQLiteAccessMethod.java index d2fd64741..f9f877166 100644 --- a/src/edu/washington/escience/myria/accessmethod/SQLiteAccessMethod.java +++ b/src/edu/washington/escience/myria/accessmethod/SQLiteAccessMethod.java @@ -228,7 +228,7 @@ public Iterator tupleBatchIteratorFromQuery(final String queryString throw new DbException(e); } - return new SQLiteTupleBatchIterator(statement, schema, sqliteConnection); + return new SQLiteTupleBatchIterator(statement, sqliteConnection, schema); } @Override diff --git a/src/edu/washington/escience/myria/accessmethod/SQLiteTupleBatchIterator.java b/src/edu/washington/escience/myria/accessmethod/SQLiteTupleBatchIterator.java index 24a6fe1b2..ac71652b5 100644 --- a/src/edu/washington/escience/myria/accessmethod/SQLiteTupleBatchIterator.java +++ b/src/edu/washington/escience/myria/accessmethod/SQLiteTupleBatchIterator.java @@ -6,6 +6,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.NoSuchElementException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,19 +35,6 @@ public class SQLiteTupleBatchIterator implements Iterator { /** The Schema of the TupleBatches returned by this Iterator. */ private final Schema schema; - /** - * Wraps a SQLiteStatement result set in an Iterator. - * - * @param statement the SQLiteStatement containing the results. - * @param schema the Schema describing the format of the TupleBatch containing these results. - * @param connection the connection to the SQLite database. - */ - SQLiteTupleBatchIterator(final SQLiteStatement statement, final Schema schema, final SQLiteConnection connection) { - this.statement = statement; - this.connection = connection; - this.schema = schema; - } - /** * Wraps a SQLiteStatement result set in an Iterator. * @@ -81,6 +69,10 @@ public boolean hasNext() { @Override public TupleBatch next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + /* Allocate TupleBatch parameters */ final int numFields = schema.numColumns(); final List> columnBuilders = ColumnFactory.allocateColumns(schema); diff --git a/src/edu/washington/escience/myria/coordinator/catalog/MasterCatalog.java b/src/edu/washington/escience/myria/coordinator/catalog/MasterCatalog.java index 84d8ab8f1..99cee1fd8 100644 --- a/src/edu/washington/escience/myria/coordinator/catalog/MasterCatalog.java +++ b/src/edu/washington/escience/myria/coordinator/catalog/MasterCatalog.java @@ -33,6 +33,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterators; import com.google.common.collect.Lists; import edu.washington.escience.myria.MyriaConstants.FTMode; @@ -2002,10 +2003,7 @@ protected Iterator job(final SQLiteConnection sqliteConnection) thro SQLiteException { SQLiteStatement statement = sqliteConnection.prepare(queryString); List tuples = Lists.newLinkedList(); - Iterator iter = new SQLiteTupleBatchIterator(statement, sqliteConnection, outputSchema); - while (iter.hasNext()) { - tuples.add(iter.next()); - } + Iterators.addAll(tuples, new SQLiteTupleBatchIterator(statement, sqliteConnection, outputSchema)); return tuples.iterator(); } }).get(); diff --git a/src/edu/washington/escience/myria/operator/CatalogQueryScan.java b/src/edu/washington/escience/myria/operator/CatalogQueryScan.java index cc3054d2b..41fd441b3 100644 --- a/src/edu/washington/escience/myria/operator/CatalogQueryScan.java +++ b/src/edu/washington/escience/myria/operator/CatalogQueryScan.java @@ -3,8 +3,6 @@ import java.util.Iterator; import java.util.Objects; -import com.google.common.collect.ImmutableMap; - import edu.washington.escience.myria.DbException; import edu.washington.escience.myria.Schema; import edu.washington.escience.myria.coordinator.catalog.CatalogException; @@ -12,8 +10,8 @@ import edu.washington.escience.myria.storage.TupleBatch; /** - * Push a select query down into a JDBC based database and scan over the query result. - * */ + * Operator to get the result of a query on the catalog. The catalog is a SQLite database. + */ public class CatalogQueryScan extends LeafOperator { /** @@ -50,14 +48,9 @@ public class CatalogQueryScan extends LeafOperator { * @param catalog see the corresponding field. * */ public CatalogQueryScan(final String sql, final Schema outputSchema, final MasterCatalog catalog) { - Objects.requireNonNull(sql); - Objects.requireNonNull(outputSchema); - Objects.requireNonNull(catalog); - - this.sql = sql; - this.outputSchema = outputSchema; - this.catalog = catalog; - tuples = null; + this.sql = Objects.requireNonNull(sql);; + this.outputSchema = Objects.requireNonNull(outputSchema); + this.catalog = Objects.requireNonNull(catalog); } @Override @@ -87,8 +80,4 @@ protected final TupleBatch fetchNextReady() throws DbException { public final Schema generateSchema() { return outputSchema; } - - @Override - protected final void init(final ImmutableMap execEnvVars) throws DbException { - } } diff --git a/src/edu/washington/escience/myria/operator/DbQueryScan.java b/src/edu/washington/escience/myria/operator/DbQueryScan.java index 0ddb932c1..4aafa858d 100644 --- a/src/edu/washington/escience/myria/operator/DbQueryScan.java +++ b/src/edu/washington/escience/myria/operator/DbQueryScan.java @@ -68,13 +68,8 @@ public class DbQueryScan extends LeafOperator implements DbReader { * @param outputSchema see the corresponding field. * */ public DbQueryScan(final String baseSQL, final Schema outputSchema) { - Objects.requireNonNull(baseSQL); - Objects.requireNonNull(outputSchema); - - this.baseSQL = baseSQL; - this.outputSchema = outputSchema; - connectionInfo = null; - tuples = null; + this.baseSQL = Objects.requireNonNull(baseSQL); + this.outputSchema = Objects.requireNonNull(outputSchema); sortedColumns = null; ascending = null; } @@ -88,8 +83,7 @@ public DbQueryScan(final String baseSQL, final Schema outputSchema) { * */ public DbQueryScan(final ConnectionInfo connectionInfo, final String baseSQL, final Schema outputSchema) { this(baseSQL, outputSchema); - Objects.requireNonNull(connectionInfo); - this.connectionInfo = connectionInfo; + this.connectionInfo = Objects.requireNonNull(connectionInfo); } /** @@ -99,14 +93,8 @@ public DbQueryScan(final ConnectionInfo connectionInfo, final String baseSQL, fi * @param outputSchema the Schema of the returned tuples. */ public DbQueryScan(final RelationKey relationKey, final Schema outputSchema) { - Objects.requireNonNull(relationKey); - Objects.requireNonNull(outputSchema); - - this.relationKey = relationKey; - this.outputSchema = outputSchema; - baseSQL = null; - connectionInfo = null; - tuples = null; + this.relationKey = Objects.requireNonNull(relationKey); + this.outputSchema = Objects.requireNonNull(outputSchema); sortedColumns = null; ascending = null; } @@ -121,8 +109,7 @@ public DbQueryScan(final RelationKey relationKey, final Schema outputSchema) { */ public DbQueryScan(final ConnectionInfo connectionInfo, final RelationKey relationKey, final Schema outputSchema) { this(relationKey, outputSchema); - Objects.requireNonNull(connectionInfo); - this.connectionInfo = connectionInfo; + this.connectionInfo = Objects.requireNonNull(connectionInfo); } /** @@ -135,16 +122,10 @@ public DbQueryScan(final ConnectionInfo connectionInfo, final RelationKey relati */ public DbQueryScan(final RelationKey relationKey, final Schema outputSchema, final int[] sortedColumns, final boolean[] ascending) { - Objects.requireNonNull(relationKey); - Objects.requireNonNull(outputSchema); - - this.relationKey = relationKey; - this.outputSchema = outputSchema; + this.relationKey = Objects.requireNonNull(relationKey); + this.outputSchema = Objects.requireNonNull(outputSchema); this.sortedColumns = sortedColumns; this.ascending = ascending; - baseSQL = null; - connectionInfo = null; - tuples = null; } /** diff --git a/test/edu/washington/escience/myria/operator/CatalogScanTest.java b/test/edu/washington/escience/myria/operator/CatalogScanTest.java index 1d8262e67..bfd9f8492 100644 --- a/test/edu/washington/escience/myria/operator/CatalogScanTest.java +++ b/test/edu/washington/escience/myria/operator/CatalogScanTest.java @@ -12,8 +12,6 @@ import org.junit.Before; import org.junit.Test; -import com.google.common.collect.ImmutableList; - import edu.washington.escience.myria.DbException; import edu.washington.escience.myria.Schema; import edu.washington.escience.myria.Type; @@ -59,7 +57,7 @@ public void Cleanup() { @Test public final void testQueryQueries() throws DbException, CatalogException { - Schema schema = new Schema(ImmutableList.of(Type.LONG_TYPE, Type.STRING_TYPE), ImmutableList.of("id", "raw")); + Schema schema = Schema.ofFields(Type.LONG_TYPE, "id", Type.STRING_TYPE, "raw"); CatalogQueryScan scan = new CatalogQueryScan("select query_id, raw_query from queries", schema, catalog); scan.open(null);