sql: make Catalog a supertrait of Transaction

Author: erikgrinaker

src/sql/engine/engine.rs

@@ -13,7 +13,7 @@ use crate::storage::mvcc;
 pub trait Engine<'a>: Sized {
     /// The engine's transaction type. This provides both row-level CRUD operations and
     /// transactional access to the schema catalog.
-    type Transaction: Transaction + Catalog + 'a;
+    type Transaction: Transaction + 'a;
 
     /// Begins a read-write transaction.
     fn begin(&'a self) -> Result<Self::Transaction>;
@@ -34,7 +34,7 @@ pub trait Engine<'a>: Sized {
 /// All methods operate on row batches rather than single rows to amortize the
 /// cost. With the Raft engine, each call results in a Raft roundtrip, and we'd
 /// rather not have to do that for every single row that's modified.
-pub trait Transaction {
+pub trait Transaction: Catalog {
     /// The transaction's internal MVCC state.
     fn state(&self) -> &mvcc::TransactionState;
 
@@ -58,13 +58,9 @@ pub trait Transaction {
 }
 
 /// The catalog stores table schema information. It must be implemented for
-/// Engine::Transaction, and is fully transactional. For simplicity, it only
+/// Transaction, and is thus fully transactional. For simplicity, it only
 /// supports creating and dropping tables -- there are no ALTER TABLE schema
 /// changes, nor CREATE INDEX.
-///
-/// This type is separate from Transaction, even though Engine::Transaction
-/// requires transactions to implement it. This allows better control of when
-/// catalog access can be used (i.e. during planning, not execution).
 pub trait Catalog {
     /// Creates a new table. Errors if it already exists.
     fn create_table(&self, table: Table) -> Result<()>;

src/sql/execution/execute.rs

@@ -1,27 +1,20 @@
 use super::{aggregate, join, source, transform, write};
 use crate::error::Result;
-use crate::sql::engine::{Catalog, Transaction};
+use crate::sql::engine::Transaction;
 use crate::sql::planner::{Node, Plan};
 use crate::sql::types::{Label, Rows};
 
 /// Executes a plan, returning an execution result.
-///
-/// Takes the transaction and catalog separately, even though Transaction must
-/// implement Catalog, to ensure the catalog is primarily used during planning.
-pub fn execute_plan(
-    plan: Plan,
-    txn: &impl Transaction,
-    catalog: &impl Catalog,
-) -> Result<ExecutionResult> {
+pub fn execute_plan(plan: Plan, txn: &impl Transaction) -> Result<ExecutionResult> {
     Ok(match plan {
         Plan::CreateTable { schema } => {
             let name = schema.name.clone();
-            catalog.create_table(schema)?;
+            txn.create_table(schema)?;
             ExecutionResult::CreateTable { name }
         }
 
         Plan::DropTable { table, if_exists } => {
-            let existed = catalog.drop_table(&table, if_exists)?;
+            let existed = txn.drop_table(&table, if_exists)?;
             ExecutionResult::DropTable { name: table, existed }
         }
 

src/sql/planner/plan.rs

@@ -67,8 +67,8 @@ impl Plan {
     }
 
     /// Executes the plan, consuming it.
-    pub fn execute(self, txn: &(impl Transaction + Catalog)) -> Result<ExecutionResult> {
-        execution::execute_plan(self, txn, txn)
+    pub fn execute(self, txn: &impl Transaction) -> Result<ExecutionResult> {
+        execution::execute_plan(self, txn)
     }
 
     /// Optimizes the plan, consuming it. See OPTIMIZERS for the list of