sql: use tuple struct for StatementResult::Begin

Author: erikgrinaker

src/bin/toysql.rs

@@ -174,7 +174,7 @@ Transactions: {active_txns} active, {versions} total
     fn execute_sql(&mut self, statement: &str) -> Result<()> {
         use StatementResult::*;
         match self.client.execute(statement)? {
-            Begin { state } => match state.read_only {
+            Begin(state) => match state.read_only {
                 true => println!("Began read-only transaction at version {}", state.version),
                 false => println!("Began transaction {}", state.version),
             },

src/client.rs

@@ -44,7 +44,7 @@ impl Client {
         };
         // Update the transaction state.
         match &result {
-            StatementResult::Begin { state } => self.txn = Some(state.clone()),
+            StatementResult::Begin(state) => self.txn = Some(state.clone()),
             StatementResult::Commit { .. } => self.txn = None,
             StatementResult::Rollback { .. } => self.txn = None,
             _ => {}

src/sql/engine/session.rs

@@ -46,7 +46,7 @@ impl<'a, E: Engine<'a>> Session<'a, E> {
                 };
                 let state = txn.state().clone();
                 self.txn = Some(txn);
-                StatementResult::Begin { state }
+                StatementResult::Begin(state)
             }
             ast::Statement::Commit => {
                 let Some(txn) = self.txn.take() else {
@@ -125,7 +125,7 @@ impl<'a, E: Engine<'a>> Drop for Session<'a, E> {
 /// A session statement result. Sent across the wire to SQL clients.
 #[derive(Debug, PartialEq, Serialize, Deserialize)]
 pub enum StatementResult {
-    Begin { state: mvcc::TransactionState },
+    Begin(mvcc::TransactionState),
     Commit { version: mvcc::Version },
     Rollback { version: mvcc::Version },
     Explain(Plan),

src/sql/testscripts/transactions/begin

@@ -11,7 +11,7 @@ c1:[result,ops]> BEGIN
 ---
 c1: set mvcc:NextVersion → 4 ["\x00" → "\x04"]
 c1: set mvcc:TxnActive(3) → "" ["\x01\x00\x00\x00\x00\x00\x00\x00\x03" → ""]
-c1: Begin { state: TransactionState { version: 3, read_only: false, active: {} } }
+c1: Begin(TransactionState { version: 3, read_only: false, active: {} })
 
 # Starting another transaction for c1 errors.
 c1:!> BEGIN
@@ -27,7 +27,7 @@ c2:[result,ops]> BEGIN
 c2: set mvcc:NextVersion → 5 ["\x00" → "\x05"]
 c2: set mvcc:TxnActiveSnapshot(4) → {3} ["\x02\x00\x00\x00\x00\x00\x00\x00\x04" → "\x01\x03"]
 c2: set mvcc:TxnActive(4) → "" ["\x01\x00\x00\x00\x00\x00\x00\x00\x04" → ""]
-c2: Begin { state: TransactionState { version: 4, read_only: false, active: {3} } }
+c2: Begin(TransactionState { version: 4, read_only: false, active: {3} })
 
 # A read-only transaction doesn't allocate a new version, and doesn't perform
 # any storage engine writes. It does capture an active set though, and it can't
@@ -36,7 +36,7 @@ c3:[result,ops]> BEGIN READ ONLY
 c3:!> INSERT INTO test VALUES (0, '')
 c3:> ROLLBACK
 ---
-c3: Begin { state: TransactionState { version: 5, read_only: true, active: {3, 4} } }
+c3: Begin(TransactionState { version: 5, read_only: true, active: {3, 4} })
 c3: Error: invalid input: primary key 0 already exists
 
 # c1 writes a value and commits.
@@ -51,7 +51,7 @@ c3:[result,ops]> BEGIN READ ONLY AS OF SYSTEM TIME 1
 c3:!> SELECT * FROM test
 c3:> ROLLBACK
 ---
-c3: Begin { state: TransactionState { version: 1, read_only: true, active: {} } }
+c3: Begin(TransactionState { version: 1, read_only: true, active: {} })
 c3: Error: invalid input: table test does not exist
 
 # It sees the table at version 2, but no rows. The row is visible
@@ -61,13 +61,13 @@ c3:[result,ops]> BEGIN READ ONLY AS OF SYSTEM TIME 2
 c3:> SELECT * FROM test
 c3:> ROLLBACK
 ---
-c3: Begin { state: TransactionState { version: 2, read_only: true, active: {} } }
+c3: Begin(TransactionState { version: 2, read_only: true, active: {} })
 
 c3:[result,ops]> BEGIN READ ONLY AS OF SYSTEM TIME 3
 c3:> SELECT * FROM test
 c3:> ROLLBACK
 ---
-c3: Begin { state: TransactionState { version: 3, read_only: true, active: {} } }
+c3: Begin(TransactionState { version: 3, read_only: true, active: {} })
 c3: 0, ''
 
 # At version 4, we inherit c2's active set which excludes c1, and still can't
@@ -76,5 +76,5 @@ c3:[result,ops]> BEGIN READ ONLY AS OF SYSTEM TIME 4
 c3:> SELECT * FROM test
 c3:> ROLLBACK
 ---
-c3: Begin { state: TransactionState { version: 4, read_only: true, active: {3} } }
+c3: Begin(TransactionState { version: 4, read_only: true, active: {3} })
 c3: 0, ''

src/sql/testscripts/transactions/commit

@@ -11,7 +11,7 @@ ok
 ---
 set mvcc:NextVersion → 3 ["\x00" → "\x03"]
 set mvcc:TxnActive(2) → "" ["\x01\x00\x00\x00\x00\x00\x00\x00\x02" → ""]
-Begin { state: TransactionState { version: 2, read_only: false, active: {} } }
+Begin(TransactionState { version: 2, read_only: false, active: {} })
 set mvcc:TxnWrite(2, sql:Row(test, 1)) → "" ["\x03\x00\x00\x00\x00\x00\x00\x00\x02\x02test\x00\xff\x00\xff\x02\x80\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x01\x00\x00" → ""]
 set mvcc:Version(sql:Row(test, 1), 2) → 1,'a' ["\x04\x02test\x00\xff\x00\xff\x02\x80\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02" → "\x01\x06\x02\x02\x02\x04\x01a"]
 set mvcc:TxnWrite(2, sql:Row(test, 2)) → "" ["\x03\x00\x00\x00\x00\x00\x00\x00\x02\x02test\x00\xff\x00\xff\x02\x80\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x02\x00\x00" → ""]
@@ -39,7 +39,7 @@ c2:[ops,result]> COMMIT
 c2: set mvcc:NextVersion → 5 ["\x00" → "\x05"]
 c2: set mvcc:TxnActiveSnapshot(4) → {3} ["\x02\x00\x00\x00\x00\x00\x00\x00\x04" → "\x01\x03"]
 c2: set mvcc:TxnActive(4) → "" ["\x01\x00\x00\x00\x00\x00\x00\x00\x04" → ""]
-c2: Begin { state: TransactionState { version: 4, read_only: false, active: {3} } }
+c2: Begin(TransactionState { version: 4, read_only: false, active: {3} })
 c2: delete mvcc:TxnActive(4) ["\x01\x00\x00\x00\x00\x00\x00\x00\x04"]
 c2: Commit { version: 4 }
 

src/sql/testscripts/transactions/isolation

@@ -38,7 +38,7 @@ ok
 c4:[result]> BEGIN
 c4:> INSERT INTO test VALUES (4, 'd')
 ---
-c4: Begin { state: TransactionState { version: 4, read_only: false, active: {2, 3} } }
+c4: Begin(TransactionState { version: 4, read_only: false, active: {2, 3} })
 
 # Commit c2.
 c2:> COMMIT

src/sql/testscripts/transactions/rollback

@@ -11,7 +11,7 @@ ok
 ---
 set mvcc:NextVersion → 3 ["\x00" → "\x03"]
 set mvcc:TxnActive(2) → "" ["\x01\x00\x00\x00\x00\x00\x00\x00\x02" → ""]
-Begin { state: TransactionState { version: 2, read_only: false, active: {} } }
+Begin(TransactionState { version: 2, read_only: false, active: {} })
 set mvcc:TxnWrite(2, sql:Row(test, 1)) → "" ["\x03\x00\x00\x00\x00\x00\x00\x00\x02\x02test\x00\xff\x00\xff\x02\x80\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x01\x00\x00" → ""]
 set mvcc:Version(sql:Row(test, 1), 2) → 1,'a' ["\x04\x02test\x00\xff\x00\xff\x02\x80\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02" → "\x01\x06\x02\x02\x02\x04\x01a"]
 set mvcc:TxnWrite(2, sql:Row(test, 2)) → "" ["\x03\x00\x00\x00\x00\x00\x00\x00\x02\x02test\x00\xff\x00\xff\x02\x80\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x00\xff\x02\x00\x00" → ""]
@@ -41,7 +41,7 @@ c2:[ops,result]> ROLLBACK
 c2: set mvcc:NextVersion → 5 ["\x00" → "\x05"]
 c2: set mvcc:TxnActiveSnapshot(4) → {3} ["\x02\x00\x00\x00\x00\x00\x00\x00\x04" → "\x01\x03"]
 c2: set mvcc:TxnActive(4) → "" ["\x01\x00\x00\x00\x00\x00\x00\x00\x04" → ""]
-c2: Begin { state: TransactionState { version: 4, read_only: false, active: {3} } }
+c2: Begin(TransactionState { version: 4, read_only: false, active: {3} })
 c2: delete mvcc:TxnActive(4) ["\x01\x00\x00\x00\x00\x00\x00\x00\x04"]
 c2: Rollback { version: 4 }
 

tests/e2e/client.rs

@@ -244,9 +244,11 @@ fn execute_txn() -> Result<()> {
     // Committing a change in a txn should work
     assert_eq!(
         c.execute("BEGIN")?,
-        StatementResult::Begin {
-            state: mvcc::TransactionState { version: 2, read_only: false, active: BTreeSet::new() }
-        }
+        StatementResult::Begin(mvcc::TransactionState {
+            version: 2,
+            read_only: false,
+            active: BTreeSet::new()
+        })
     );
     assert_eq!(
         c.txn(),
@@ -264,9 +266,11 @@ fn execute_txn() -> Result<()> {
     // Rolling back a change in a txn should also work
     assert_eq!(
         c.execute("BEGIN")?,
-        StatementResult::Begin {
-            state: mvcc::TransactionState { version: 3, read_only: false, active: BTreeSet::new() }
-        }
+        StatementResult::Begin(mvcc::TransactionState {
+            version: 3,
+            read_only: false,
+            active: BTreeSet::new()
+        })
     );
     assert_eq!(
         c.txn(),
@@ -284,9 +288,11 @@ fn execute_txn() -> Result<()> {
     // Starting a read-only txn should block writes
     assert_eq!(
         c.execute("BEGIN READ ONLY")?,
-        StatementResult::Begin {
-            state: mvcc::TransactionState { version: 4, read_only: true, active: BTreeSet::new() }
-        }
+        StatementResult::Begin(mvcc::TransactionState {
+            version: 4,
+            read_only: true,
+            active: BTreeSet::new()
+        })
     );
     assert_eq!(
         c.txn(),
@@ -307,9 +313,11 @@ fn execute_txn() -> Result<()> {
     // block writes
     assert_eq!(
         c.execute("BEGIN READ ONLY AS OF SYSTEM TIME 2")?,
-        StatementResult::Begin {
-            state: mvcc::TransactionState { version: 2, read_only: true, active: BTreeSet::new() }
-        },
+        StatementResult::Begin(mvcc::TransactionState {
+            version: 2,
+            read_only: true,
+            active: BTreeSet::new()
+        }),
     );
     assert_eq!(
         c.txn(),
@@ -329,9 +337,11 @@ fn execute_txn() -> Result<()> {
     // A txn should still be usable after an error occurs
     assert_eq!(
         c.execute("BEGIN")?,
-        StatementResult::Begin {
-            state: mvcc::TransactionState { version: 4, read_only: false, active: BTreeSet::new() }
-        },
+        StatementResult::Begin(mvcc::TransactionState {
+            version: 4,
+            read_only: false,
+            active: BTreeSet::new()
+        })
     );
     c.execute("INSERT INTO genres VALUES (5, 'Horror')")?;
     assert_eq!(
@@ -369,19 +379,19 @@ fn execute_txn_concurrent() -> Result<()> {
     // Concurrent updates should throw a serialization failure on conflict.
     assert_eq!(
         a.execute("BEGIN")?,
-        StatementResult::Begin {
-            state: mvcc::TransactionState { version: 2, read_only: false, active: BTreeSet::new() }
-        },
+        StatementResult::Begin(mvcc::TransactionState {
+            version: 2,
+            read_only: false,
+            active: BTreeSet::new()
+        })
     );
     assert_eq!(
         b.execute("BEGIN")?,
-        StatementResult::Begin {
-            state: mvcc::TransactionState {
-                version: 3,
-                read_only: false,
-                active: BTreeSet::from([2])
-            }
-        },
+        StatementResult::Begin(mvcc::TransactionState {
+            version: 3,
+            read_only: false,
+            active: BTreeSet::from([2])
+        })
     );
 
     assert_row(