Convert EDN transaction tests to Rust code. Fixes #271. (#364) r=rnewman

* Pre: Order datoms deterministically in debug output.

This makes comparison much easier, and avoids a whole class of
difficult problems when introducing pattern matching with placeholder
values.

* Pre: Don't rewrite ?txN and ?msN in debug module into_edn() methods.

* Convert EDN transaction tests to Rust code. Fixes #271.

This implements
https://github.com/mozilla/mentat/issues/271#issuecomment-283125963.
I'm using the EDN pattern matching functionality
internally (extensively!), but specifically working around the tricky
edges we encountered.  This should let us implement tests quickly (and
hopefully legibly) while not requiring us to encode as much behaviour
into non-standard EDN notations.

db/src/db.rs

@@ -858,10 +858,81 @@ mod tests {
     use bootstrap;
     use debug;
     use edn;
-    use edn::symbols;
     use mentat_tx_parser;
     use rusqlite;
-    use tx::transact;
+    use types::TxReport;
+    use tx;
+
+    // Macro to parse a `Borrow<str>` to an `edn::Value` and assert the given `edn::Value` `matches`
+    // against it.
+    //
+    // This is a macro only to give nice line numbers when tests fail.
+    macro_rules! assert_matches {
+        ( $input: expr, $expected: expr ) => {{
+            // Failure to parse the expected pattern is a coding error, so we unwrap.
+            let pattern_value = edn::parse::value($expected.borrow()).unwrap().without_spans();
+            assert!($input.matches(&pattern_value),
+                    "Expected value:\n{}\nto match pattern:\n{}\n",
+                    $input.to_pretty(120).unwrap(),
+                    pattern_value.to_pretty(120).unwrap());
+        }}
+    }
+
+    // A connection that doesn't try to be clever about possibly sharing its `Schema`.  Compare to
+    // `mentat::Conn`.
+    struct TestConn {
+        sqlite: rusqlite::Connection,
+        partition_map: PartitionMap,
+        schema: Schema,
+    }
+
+    impl TestConn {
+        fn transact<I>(&mut self, transaction: I) -> Result<TxReport> where I: Borrow<str> {
+            // Failure to parse the transaction is a coding error, so we unwrap.
+            let assertions = edn::parse::value(transaction.borrow()).unwrap().without_spans();
+            let entities: Vec<_> = mentat_tx_parser::Tx::parse(&[assertions][..]).unwrap();
+            // Applying the transaction can fail, so we don't unwrap.
+            let details = tx::transact(&self.sqlite, self.partition_map.clone(), &self.schema, entities)?;
+            let (report, next_partition_map, next_schema) = details;
+            self.partition_map = next_partition_map;
+            if let Some(next_schema) = next_schema {
+                self.schema = next_schema;
+            }
+            Ok(report)
+        }
+
+        fn last_tx_id(&self) -> Entid {
+            self.partition_map.get(&":db.part/tx".to_string()).unwrap().index - 1
+        }
+
+        fn last_transaction(&self) -> edn::Value {
+            debug::transactions_after(&self.sqlite, &self.schema, self.last_tx_id() - 1).unwrap().0[0].into_edn()
+        }
+
+        fn datoms(&self) -> edn::Value {
+            debug::datoms_after(&self.sqlite, &self.schema, bootstrap::TX0).unwrap().into_edn()
+        }
+    }
+
+    impl Default for TestConn {
+        fn default() -> TestConn {
+            let mut conn = new_connection("").expect("Couldn't open in-memory db");
+            let db = ensure_current_version(&mut conn).unwrap();
+
+            // Does not include :db/txInstant.
+            let datoms = debug::datoms_after(&conn, &db.schema, 0).unwrap();
+            assert_eq!(datoms.0.len(), 88);
+
+            // Includes :db/txInstant.
+            let transactions = debug::transactions_after(&conn, &db.schema, 0).unwrap();
+            assert_eq!(transactions.0.len(), 1);
+            assert_eq!(transactions.0[0].0.len(), 89);
+
+            TestConn { sqlite: conn,
+                       partition_map: db.partition_map,
+                       schema: db.schema }
+        }
+    }
 
     #[test]
     fn test_open_current_version() {
@@ -887,136 +958,217 @@ mod tests {
         assert_eq!(transactions.0[0].0.len(), 89);
     }
 
-    /// Assert that a sequence of transactions meets expectations.
-    ///
-    /// The transactions, expectations, and optional labels, are given in a simple EDN format; see
-    /// https://github.com/mozilla/mentat/wiki/Transacting:-EDN-test-format.
-    ///
-    /// There is some magic here about transaction numbering that I don't want to commit to or
-    /// document just yet.  The end state might be much more general pattern matching syntax, rather
-    /// than the targeted transaction ID and timestamp replacement we have right now.
-    // TODO: accept a `Conn`.
-    fn assert_transactions<'a>(conn: &rusqlite::Connection, partition_map: &mut PartitionMap, schema: &mut Schema, transactions: &Vec<edn::Value>) {
-        let mut index: i64 = bootstrap::TX0;
-
-        for transaction in transactions {
-            let transaction = transaction.as_map().unwrap();
-            let label: edn::Value = transaction.get(&edn::Value::NamespacedKeyword(symbols::NamespacedKeyword::new("test", "label"))).unwrap().clone();
-            let assertions: edn::Value = transaction.get(&edn::Value::NamespacedKeyword(symbols::NamespacedKeyword::new("test", "assertions"))).unwrap().clone();
-            let expected_transaction: Option<&edn::Value> = transaction.get(&edn::Value::NamespacedKeyword(symbols::NamespacedKeyword::new("test", "expected-transaction")));
-            let expected_datoms: Option<&edn::Value> = transaction.get(&edn::Value::NamespacedKeyword(symbols::NamespacedKeyword::new("test", "expected-datoms")));
-            let expected_error_message: Option<&edn::Value> = transaction.get(&edn::Value::NamespacedKeyword(symbols::NamespacedKeyword::new("test", "expected-error-message")));
-
-            let entities: Vec<_> = mentat_tx_parser::Tx::parse(&[assertions][..]).unwrap();
-
-            let maybe_report = transact(&conn, partition_map.clone(), schema, entities);
-
-            if let Some(expected_transaction) = expected_transaction {
-                if !expected_transaction.is_nil() {
-                    index += 1;
-                } else {
-                    assert!(maybe_report.is_err());
-
-                    if let Some(expected_error_message) = expected_error_message {
-                        let expected_error_message = expected_error_message.as_text();
-                        assert!(expected_error_message.is_some(), "Expected error message to be text:\n{:?}", expected_error_message);
-                        let error_message = maybe_report.unwrap_err().to_string();
-                        assert!(error_message.contains(expected_error_message.unwrap()), "Expected error message:\n{}\nto contain:\n{}", error_message, expected_error_message.unwrap());
-                    }
-                    continue
-                }
-
-                // TODO: test schema changes in the EDN format.
-                let (report, next_partition_map, next_schema) = maybe_report.unwrap();
-                *partition_map = next_partition_map;
-                if let Some(next_schema) = next_schema {
-                    *schema = next_schema;
-                }
-
-                assert_eq!(index, report.tx_id,
-                           "\n{} - expected tx_id {} but got tx_id {}", label, index, report.tx_id);
-
-                let transactions = debug::transactions_after(&conn, &schema, index - 1).unwrap();
-                assert_eq!(transactions.0.len(), 1);
-                assert_eq!(*expected_transaction,
-                           transactions.0[0].into_edn(),
-                           "\n{} - expected transaction:\n{}\nbut got transaction:\n{}", label, *expected_transaction, transactions.0[0].into_edn());
-            }
-
-            if let Some(expected_datoms) = expected_datoms {
-                let datoms = debug::datoms_after(&conn, &schema, bootstrap::TX0).unwrap();
-                assert_eq!(*expected_datoms,
-                           datoms.into_edn(),
-                           "\n{} - expected datoms:\n{}\nbut got datoms:\n{}", label, *expected_datoms, datoms.into_edn())
-            }
-
-            // Don't allow empty tests.  This will need to change if we allow transacting schema
-            // fragments in a preamble, but for now it might catch malformed tests.
-            assert_ne!((expected_transaction, expected_datoms), (None, None),
-                       "Transaction test must include at least one of :test/expected-transaction or :test/expected-datoms");
-        }
-    }
-
     #[test]
     fn test_add() {
-        let mut conn = new_connection("").expect("Couldn't open in-memory db");
+        let mut conn = TestConn::default();
-        let mut db = ensure_current_version(&mut conn).unwrap();
 
-        // Does not include :db/txInstant.
+        // Test inserting :db.cardinality/one elements.
-        let datoms = debug::datoms_after(&conn, &db.schema, 0).unwrap();
+        conn.transact("[[:db/add 100 :db/ident :keyword/value1]
-        assert_eq!(datoms.0.len(), 88);
+                        [:db/add 101 :db/ident :keyword/value2]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[100 :db/ident :keyword/value1 ?tx true]
+                          [101 :db/ident :keyword/value2 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                       "[[100 :db/ident :keyword/value1]
+                         [101 :db/ident :keyword/value2]]");
 
-        // Includes :db/txInstant.
+        // Test inserting :db.cardinality/many elements.
-        let transactions = debug::transactions_after(&conn, &db.schema, 0).unwrap();
+        conn.transact("[[:db/add 200 :db.schema/attribute 100]
-        assert_eq!(transactions.0.len(), 1);
+                        [:db/add 200 :db.schema/attribute 101]]").unwrap();
-        assert_eq!(transactions.0[0].0.len(), 89);
+        assert_matches!(conn.last_transaction(),
+                        "[[200 :db.schema/attribute 100 ?tx true]
+                          [200 :db.schema/attribute 101 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :keyword/value1]
+                          [101 :db/ident :keyword/value2]
+                          [200 :db.schema/attribute 100]
+                          [200 :db.schema/attribute 101]]");
 
-        // TODO: extract a test macro simplifying this boilerplate yet further.
+        // Test replacing existing :db.cardinality/one elements.
-        let value = edn::parse::value(include_str!("../../tx/fixtures/test_add.edn")).unwrap().without_spans();
+        conn.transact("[[:db/add 100 :db/ident :keyword/value11]
+                        [:db/add 101 :db/ident :keyword/value22]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[100 :db/ident :keyword/value1 ?tx false]
+                          [100 :db/ident :keyword/value11 ?tx true]
+                          [101 :db/ident :keyword/value2 ?tx false]
+                          [101 :db/ident :keyword/value22 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :keyword/value11]
+                          [101 :db/ident :keyword/value22]
+                          [200 :db.schema/attribute 100]
+                          [200 :db.schema/attribute 101]]");
 
-        let transactions = value.as_vector().unwrap();
 
-        assert_transactions(&conn, &mut db.partition_map, &mut db.schema, transactions);
+        // Test that asserting existing :db.cardinality/one elements doesn't change the store.
+        conn.transact("[[:db/add 100 :db/ident :keyword/value11]
+                        [:db/add 101 :db/ident :keyword/value22]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :keyword/value11]
+                          [101 :db/ident :keyword/value22]
+                          [200 :db.schema/attribute 100]
+                          [200 :db.schema/attribute 101]]");
+
+
+        // Test that asserting existing :db.cardinality/many elements doesn't change the store.
+        conn.transact("[[:db/add 200 :db.schema/attribute 100]
+                        [:db/add 200 :db.schema/attribute 101]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :keyword/value11]
+                          [101 :db/ident :keyword/value22]
+                          [200 :db.schema/attribute 100]
+                          [200 :db.schema/attribute 101]]");
     }
 
     #[test]
     fn test_retract() {
-        let mut conn = new_connection("").expect("Couldn't open in-memory db");
+        let mut conn = TestConn::default();
-        let mut db = ensure_current_version(&mut conn).unwrap();
 
-        // Does not include :db/txInstant.
+        // Insert a few :db.cardinality/one elements.
-        let datoms = debug::datoms_after(&conn, &db.schema, 0).unwrap();
+        conn.transact("[[:db/add 100 :db/ident :keyword/value1]
-        assert_eq!(datoms.0.len(), 88);
+                        [:db/add 101 :db/ident :keyword/value2]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[100 :db/ident :keyword/value1 ?tx true]
+                          [101 :db/ident :keyword/value2 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :keyword/value1]
+                          [101 :db/ident :keyword/value2]]");
 
-        // Includes :db/txInstant.
+        // And a few :db.cardinality/many elements.
-        let transactions = debug::transactions_after(&conn, &db.schema, 0).unwrap();
+        conn.transact("[[:db/add 200 :db.schema/attribute 100]
-        assert_eq!(transactions.0.len(), 1);
+                        [:db/add 200 :db.schema/attribute 101]]").unwrap();
-        assert_eq!(transactions.0[0].0.len(), 89);
+        assert_matches!(conn.last_transaction(),
+                        "[[200 :db.schema/attribute 100 ?tx true]
+                          [200 :db.schema/attribute 101 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :keyword/value1]
+                          [101 :db/ident :keyword/value2]
+                          [200 :db.schema/attribute 100]
+                          [200 :db.schema/attribute 101]]");
 
-        let value = edn::parse::value(include_str!("../../tx/fixtures/test_retract.edn")).unwrap().without_spans();
+        // Test that we can retract :db.cardinality/one elements.
+        conn.transact("[[:db/retract 100 :db/ident :keyword/value1]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[100 :db/ident :keyword/value1 ?tx false]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[101 :db/ident :keyword/value2]
+                          [200 :db.schema/attribute 100]
+                          [200 :db.schema/attribute 101]]");
 
-        let transactions = value.as_vector().unwrap();
+        // Test that we can retract :db.cardinality/many elements.
-        assert_transactions(&conn, &mut db.partition_map, &mut db.schema, transactions);
+        conn.transact("[[:db/retract 200 :db.schema/attribute 100]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[200 :db.schema/attribute 100 ?tx false]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[101 :db/ident :keyword/value2]
+                          [200 :db.schema/attribute 101]]");
+
+        // Verify that retracting :db.cardinality/{one,many} elements that are not present doesn't
+        // change the store.
+        conn.transact("[[:db/retract 100 :db/ident :keyword/value1]
+                        [:db/retract 200 :db.schema/attribute 100]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[101 :db/ident :keyword/value2]
+                          [200 :db.schema/attribute 101]]");
     }
 
     #[test]
     fn test_upsert_vector() {
-        let mut conn = new_connection("").expect("Couldn't open in-memory db");
+        // TODO: assert the tempids allocated throughout.
-        let mut db = ensure_current_version(&mut conn).unwrap();
+        let mut conn = TestConn::default();
 
-        // Does not include :db/txInstant.
+        // Insert some :db.unique/identity elements.
-        let datoms = debug::datoms_after(&conn, &db.schema, 0).unwrap();
+        conn.transact("[[:db/add 100 :db/ident :name/Ivan]
-        assert_eq!(datoms.0.len(), 88);
+                        [:db/add 101 :db/ident :name/Petr]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[100 :db/ident :name/Ivan ?tx true]
+                          [101 :db/ident :name/Petr ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :name/Ivan]
+                          [101 :db/ident :name/Petr]]");
 
-        // Includes :db/txInstant.
+        // Upserting two tempids to the same entid works.
-        let transactions = debug::transactions_after(&conn, &db.schema, 0).unwrap();
+        conn.transact("[[:db/add \"t1\" :db/ident :name/Ivan]
-        assert_eq!(transactions.0.len(), 1);
+                        [:db/add \"t1\" :db.schema/attribute 100]
-        assert_eq!(transactions.0[0].0.len(), 89);
+                        [:db/add \"t2\" :db/ident :name/Petr]
+                        [:db/add \"t2\" :db.schema/attribute 101]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[100 :db.schema/attribute 100 ?tx true]
+                          [101 :db.schema/attribute 101 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :name/Ivan]
+                          [100 :db.schema/attribute 100]
+                          [101 :db/ident :name/Petr]
+                          [101 :db.schema/attribute 101]]");
 
-        let value = edn::parse::value(include_str!("../../tx/fixtures/test_upsert_vector.edn")).unwrap().without_spans();
+        // Upserting a tempid works.  The ref doesn't have to exist (at this time), but we can't
+        // reuse an existing ref due to :db/unique :db.unique/value.
+        conn.transact("[[:db/add \"t1\" :db/ident :name/Ivan]
+                        [:db/add \"t1\" :db.schema/attribute 102]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[100 :db.schema/attribute 102 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(conn.datoms(),
+                        "[[100 :db/ident :name/Ivan]
+                          [100 :db.schema/attribute 100]
+                          [100 :db.schema/attribute 102]
+                          [101 :db/ident :name/Petr]
+                          [101 :db.schema/attribute 101]]");
 
-        let transactions = value.as_vector().unwrap();
+        // A single complex upsert allocates a new entid.
-        assert_transactions(&conn, &mut db.partition_map, &mut db.schema, transactions);
+        conn.transact("[[:db/add \"t1\" :db.schema/attribute \"t2\"]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[65536 :db.schema/attribute 65537 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+
+        // Conflicting upserts fail.
+        let err = conn.transact("[[:db/add \"t1\" :db/ident :name/Ivan]
+                                  [:db/add \"t1\" :db/ident :name/Petr]]").unwrap_err().to_string();
+        assert_eq!(err, "not yet implemented: Conflicting upsert: tempid \'t1\' resolves to more than one entid: 100, 101");
+
+        // tempids in :db/retract that don't upsert fail.
+        let err = conn.transact("[[:db/retract \"t1\" :db/ident :name/Anonymous]]").unwrap_err().to_string();
+        assert_eq!(err, "not yet implemented: [:db/retract ...] entity referenced tempid that did not upsert: t1");
+
+        // tempids in :db/retract that do upsert are retracted.  The ref given doesn't exist, so the
+        // assertion will be ignored.
+        conn.transact("[[:db/add \"t1\" :db/ident :name/Ivan]
+                        [:db/retract \"t1\" :db.schema/attribute 103]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[?tx :db/txInstant ?ms ?tx true]]");
+
+        // A multistep upsert.  The upsert algorithm will first try to resolve "t1", fail, and then
+        // allocate both "t1" and "t2".
+        conn.transact("[[:db/add \"t1\" :db/ident :name/Josef]
+                        [:db/add \"t2\" :db.schema/attribute \"t1\"]]").unwrap();
+        assert_matches!(conn.last_transaction(),
+                        "[[65538 :db/ident :name/Josef ?tx true]
+                          [65539 :db.schema/attribute 65538 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+
+        // A multistep insert.  This time, we can resolve both, but we have to try "t1", succeed,
+        // and then resolve "t2".
+        // TODO: We can't quite test this without more schema elements.
+        // conn.transact("[[:db/add \"t1\" :db/ident :name/Josef]
+        //                 [:db/add \"t2\" :db/ident \"t1\"]]");
+        // assert_matches!(conn.last_transaction(),
+        //                 "[[65538 :db/ident :name/Josef]
+        //                   [65538 :db/ident :name/Karl]
+        //                   [?tx :db/txInstant ?ms ?tx true]]");
     }
 
     #[test]

db/src/debug.rs

@@ -13,7 +13,6 @@
 /// Low-level functions for testing.
 
 use std::borrow::Borrow;
-use std::collections::{BTreeSet};
 use std::io::{Write};
 
 use itertools::Itertools;
@@ -21,10 +20,8 @@ use rusqlite;
 use rusqlite::types::{ToSql};
 use tabwriter::TabWriter;
 
-use ::{to_namespaced_keyword};
 use bootstrap;
 use edn;
-use edn::symbols;
 use entids;
 use mentat_core::TypedValue;
 use mentat_tx::entities::{Entid};
@@ -44,22 +41,22 @@ pub struct Datom {
 }
 
 /// Represents a set of datoms (assertions) in the store.
-pub struct Datoms(pub BTreeSet<Datom>);
+///
+/// To make comparision easier, we deterministically order.  The ordering is the ascending tuple
+/// ordering determined by `(e, a, (value_type_tag, v), tx)`, where `value_type_tag` is an internal
+/// value that is not exposed but is deterministic.
+pub struct Datoms(pub Vec<Datom>);
 
 /// Represents an ordered sequence of transactions in the store.
+///
+/// To make comparision easier, we deterministically order.  The ordering is the ascending tuple
+/// ordering determined by `(e, a, (value_type_tag, v), tx, added)`, where `value_type_tag` is an
+/// internal value that is not exposed but is deterministic, and `added` is ordered such that
+/// retracted assertions appear before added assertions.
 pub struct Transactions(pub Vec<Datoms>);
 
-fn label_tx_id(tx: i64) -> edn::Value {
-    edn::Value::PlainSymbol(symbols::PlainSymbol::new(format!("?tx{}", tx - bootstrap::TX0)))
-}
-
-fn label_tx_instant(tx: i64) -> edn::Value {
-    edn::Value::PlainSymbol(symbols::PlainSymbol::new(format!("?ms{}", tx - bootstrap::TX0)))
-}
-
 impl Datom {
-    pub fn into_edn<T, U>(&self, tx_id: T, tx_instant: &U) -> edn::Value
+    pub fn into_edn(&self) -> edn::Value {
-        where T: Fn(i64) -> edn::Value, U: Fn(i64) -> edn::Value {
         let f = |entid: &Entid| -> edn::Value {
             match *entid {
                 Entid::Entid(ref y) => edn::Value::Integer(y.clone()),
@@ -67,16 +64,9 @@ impl Datom {
             }
         };
 
-        // Rewrite [E :db/txInstant V] to [?txN :db/txInstant ?t0].
+        let mut v = vec![f(&self.e), f(&self.a), self.v.clone()];
-        let mut v = if self.a == Entid::Entid(entids::DB_TX_INSTANT) || self.a == Entid::Ident(to_namespaced_keyword(":db/txInstant").unwrap()) {
-            vec![tx_id(self.tx),
-                 f(&self.a),
-                 tx_instant(self.tx)]
-        } else {
-            vec![f(&self.e), f(&self.a), self.v.clone()]
-        };
         if let Some(added) = self.added {
-            v.push(tx_id(self.tx));
+            v.push(edn::Value::Integer(self.tx));
             v.push(edn::Value::Boolean(added));
         }
 
@@ -85,24 +75,14 @@ impl Datom {
 }
 
 impl Datoms {
-    pub fn into_edn_raw<T, U>(&self, tx_id: &T, tx_instant: &U) -> edn::Value
-        where T: Fn(i64) -> edn::Value, U: Fn(i64) -> edn::Value {
-        edn::Value::Set((&self.0).into_iter().map(|x| x.into_edn(tx_id, tx_instant)).collect())
-    }
-
     pub fn into_edn(&self) -> edn::Value {
-        self.into_edn_raw(&label_tx_id, &label_tx_instant)
+        edn::Value::Vector((&self.0).into_iter().map(|x| x.into_edn()).collect())
     }
 }
 
 impl Transactions {
-    pub fn into_edn_raw<T, U>(&self, tx_id: &T, tx_instant: &U) -> edn::Value
-        where T: Fn(i64) -> edn::Value, U: Fn(i64) -> edn::Value {
-        edn::Value::Vector((&self.0).into_iter().map(|x| x.into_edn_raw(tx_id, tx_instant)).collect())
-    }
-
     pub fn into_edn(&self) -> edn::Value {
-        self.into_edn_raw(&label_tx_id, &label_tx_instant)
+        edn::Value::Vector((&self.0).into_iter().map(|x| x.into_edn()).collect())
     }
 }
 
@@ -122,7 +102,7 @@ pub fn datoms<S: Borrow<Schema>>(conn: &rusqlite::Connection, schema: &S) -> Res
 ///
 /// The datom set returned does not include any datoms of the form [... :db/txInstant ...].
 pub fn datoms_after<S: Borrow<Schema>>(conn: &rusqlite::Connection, schema: &S, tx: i64) -> Result<Datoms> {
-    let mut stmt: rusqlite::Statement = conn.prepare("SELECT e, a, v, value_type_tag, tx FROM datoms WHERE tx > ? ORDER BY e ASC, a ASC, v ASC, tx ASC")?;
+    let mut stmt: rusqlite::Statement = conn.prepare("SELECT e, a, v, value_type_tag, tx FROM datoms WHERE tx > ? ORDER BY e ASC, a ASC, value_type_tag ASC, v ASC, tx ASC")?;
 
     let r: Result<Vec<_>> = stmt.query_and_then(&[&tx], |row| {
         let e: i64 = row.get_checked(0)?;
@@ -142,7 +122,7 @@ pub fn datoms_after<S: Borrow<Schema>>(conn: &rusqlite::Connection, schema: &S,
 
         let borrowed_schema = schema.borrow();
         Ok(Some(Datom {
-            e: to_entid(borrowed_schema, e),
+            e: Entid::Entid(e),
             a: to_entid(borrowed_schema, a),
             v: value,
             tx: tx,
@@ -158,7 +138,7 @@ pub fn datoms_after<S: Borrow<Schema>>(conn: &rusqlite::Connection, schema: &S,
 ///
 /// Each transaction returned includes the [:db/tx :db/txInstant ...] datom.
 pub fn transactions_after<S: Borrow<Schema>>(conn: &rusqlite::Connection, schema: &S, tx: i64) -> Result<Transactions> {
-    let mut stmt: rusqlite::Statement = conn.prepare("SELECT e, a, v, value_type_tag, tx, added FROM transactions WHERE tx > ? ORDER BY tx ASC, e ASC, a ASC, v ASC, added ASC")?;
+    let mut stmt: rusqlite::Statement = conn.prepare("SELECT e, a, v, value_type_tag, tx, added FROM transactions WHERE tx > ? ORDER BY tx ASC, e ASC, a ASC, value_type_tag ASC, v ASC, added ASC")?;
 
     let r: Result<Vec<_>> = stmt.query_and_then(&[&tx], |row| {
         let e: i64 = row.get_checked(0)?;
@@ -175,7 +155,7 @@ pub fn transactions_after<S: Borrow<Schema>>(conn: &rusqlite::Connection, schema
 
         let borrowed_schema = schema.borrow();
         Ok(Datom {
-            e: to_entid(borrowed_schema, e),
+            e: Entid::Entid(e),
             a: to_entid(borrowed_schema, a),
             v: value,
             tx: tx,

tx/fixtures/test_add.edn

@@ -1,66 +0,0 @@
-[{:test/label ":db.cardinality/one, insert"
-  :test/assertions
-  [[:db/add 100 :db/ident :keyword/value1]
-   [:db/add 101 :db/ident :keyword/value2]]
-  :test/expected-transaction
-  #{[100 :db/ident :keyword/value1 ?tx1 true]
-    [101 :db/ident :keyword/value2 ?tx1 true]
-    [?tx1 :db/txInstant ?ms1 ?tx1 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :keyword/value1]
-    [101 :db/ident :keyword/value2]}}
-
- {:test/label ":db.cardinality/many, insert"
-  :test/assertions
-  [[:db/add 200 :db.schema/attribute 100]
-   [:db/add 200 :db.schema/attribute 101]]
-  :test/expected-transaction
-  #{[200 :db.schema/attribute 100 ?tx2 true]
-    [200 :db.schema/attribute 101 ?tx2 true]
-    [?tx2 :db/txInstant ?ms2 ?tx2 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :keyword/value1]
-    [101 :db/ident :keyword/value2]
-    [200 :db.schema/attribute 100]
-    [200 :db.schema/attribute 101]}}
-
- {:test/label ":db.cardinality/one, replace"
-  :test/assertions
-  [[:db/add 100 :db/ident :keyword/value11]
-   [:db/add 101 :db/ident :keyword/value22]]
-  :test/expected-transaction
-  #{[100 :db/ident :keyword/value1 ?tx3 false]
-    [100 :db/ident :keyword/value11 ?tx3 true]
-    [101 :db/ident :keyword/value2 ?tx3 false]
-    [101 :db/ident :keyword/value22 ?tx3 true]
-    [?tx3 :db/txInstant ?ms3 ?tx3 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :keyword/value11]
-    [101 :db/ident :keyword/value22]
-    [200 :db.schema/attribute 100]
-    [200 :db.schema/attribute 101]}}
-
- {:test/label ":db.cardinality/one, already present"
-  :test/assertions
-  [[:db/add 100 :db/ident :keyword/value11]
-   [:db/add 101 :db/ident :keyword/value22]]
-  :test/expected-transaction
-  #{[?tx4 :db/txInstant ?ms4 ?tx4 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :keyword/value11]
-    [101 :db/ident :keyword/value22]
-    [200 :db.schema/attribute 100]
-    [200 :db.schema/attribute 101]}}
-
- {:test/label ":db.cardinality/many, already present"
-  :test/assertions
-  [[:db/add 200 :db.schema/attribute 100]
-   [:db/add 200 :db.schema/attribute 101]]
-  :test/expected-transaction
-  #{[?tx5 :db/txInstant ?ms5 ?tx5 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :keyword/value11]
-    [101 :db/ident :keyword/value22]
-    [200 :db.schema/attribute 100]
-    [200 :db.schema/attribute 101]}}
- ]

tx/fixtures/test_retract.edn

@@ -1,59 +0,0 @@
-[{:test/label ":db.cardinality/one, insert"
-  :test/assertions
-  [[:db/add 100 :db/ident :keyword/value1]
-   [:db/add 101 :db/ident :keyword/value2]]
-  :test/expected-transaction
-  #{[100 :db/ident :keyword/value1 ?tx1 true]
-    [101 :db/ident :keyword/value2 ?tx1 true]
-    [?tx1 :db/txInstant ?ms1 ?tx1 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :keyword/value1]
-    [101 :db/ident :keyword/value2]}}
-
- {:test/label ":db.cardinality/many, insert"
-  :test/assertions
-  [[:db/add 200 :db.schema/attribute 100]
-   [:db/add 200 :db.schema/attribute 101]]
-  :test/expected-transaction
-  #{[200 :db.schema/attribute 100 ?tx2 true]
-    [200 :db.schema/attribute 101 ?tx2 true]
-    [?tx2 :db/txInstant ?ms2 ?tx2 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :keyword/value1]
-    [101 :db/ident :keyword/value2]
-    [200 :db.schema/attribute 100]
-    [200 :db.schema/attribute 101]}}
-
- {:test/label ":db.cardinality/one, retract"
-  :test/assertions
-  [[:db/retract 100 :db/ident :keyword/value1]]
-  :test/expected-transaction
-  #{[100 :db/ident :keyword/value1 ?tx3 false]
-    [?tx3 :db/txInstant ?ms3 ?tx3 true]}
-  :test/expected-datoms
-  #{[101 :db/ident :keyword/value2]
-    [200 :db.schema/attribute 100]
-    [200 :db.schema/attribute 101]}}
-
- {:test/label ":db.cardinality/many, retract"
-  :test/assertions
-  [[:db/retract 200 :db.schema/attribute 100]]
-  :test/expected-transaction
-  #{[200 :db.schema/attribute 100 ?tx4 false]
-    [?tx4 :db/txInstant ?ms4 ?tx4 true]}
-  :test/expected-datoms
-  #{[101 :db/ident :keyword/value2]
-    [200 :db.schema/attribute 101]}
-  }
-
- {:test/label ":db.cardinality/{one,many}, not present."
-  :test/assertions
-  [[:db/retract 100 :db/ident :keyword/value1]
-   [:db/retract 200 :db.schema/attribute 100]]
-  :test/expected-transaction
-  #{[?tx5 :db/txInstant ?ms5 ?tx5 true]}
-  :test/expected-datoms
-  #{[101 :db/ident :keyword/value2]
-    [200 :db.schema/attribute 101]}
-  }
- ]

tx/fixtures/test_upsert_vector.edn

@@ -1,121 +0,0 @@
-[{:test/label ":db.cardinality/one, insert"
-  :test/assertions
-  [[:db/add 100 :db/ident :name/Ivan]
-   [:db/add 101 :db/ident :name/Petr]]
-  :test/expected-transaction
-  #{[100 :db/ident :name/Ivan ?tx1 true]
-    [101 :db/ident :name/Petr ?tx1 true]
-    [?tx1 :db/txInstant ?ms1 ?tx1 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :name/Ivan]
-    [101 :db/ident :name/Petr]}}
-
- {:test/label "upsert two tempids to same entid"
-  :test/assertions
-  [[:db/add "t1" :db/ident :name/Ivan]
-   [:db/add "t1" :db.schema/attribute 100]
-   [:db/add "t2" :db/ident :name/Petr]
-   [:db/add "t2" :db.schema/attribute 101]]
-  :test/expected-transaction
-  #{[100 :db.schema/attribute 100 ?tx2 true]
-    [101 :db.schema/attribute 101 ?tx2 true]
-    [?tx2 :db/txInstant ?ms2 ?tx2 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :name/Ivan]
-    [101 :db/ident :name/Petr]
-    [100 :db.schema/attribute 100]
-    [101 :db.schema/attribute 101]}
-  :test/expected-tempids
-  {"t1" 100
-   "t2" 101}}
-
- {:test/label "upsert with tempid"
-  :test/assertions
-  [[:db/add "t1" :db/ident :name/Ivan]
-   ;; Ref doesn't have to exist (at this time).  Can't reuse due to :db/unique :db.unique/value.
-   [:db/add "t1" :db.schema/attribute 102]]
-  :test/expected-transaction
-  #{[100 :db.schema/attribute 102 ?tx3 true]
-    [?tx3 :db/txInstant ?ms3 ?tx3 true]}
-  :test/expected-datoms
-  #{[100 :db/ident :name/Ivan]
-    [101 :db/ident :name/Petr]
-    [100 :db.schema/attribute 100]
-    [100 :db.schema/attribute 102]
-    [101 :db.schema/attribute 101]}
-  :test/expected-tempids
-  {"t1" 100}}
-
- ;; TODO: don't hard-code allocated entids.
- {:test/label "single complex upsert allocates new entid"
-  :test/assertions
-  [[:db/add "t1" :db.schema/attribute "t2"]]
-  :test/expected-transaction
-  #{[65536 :db.schema/attribute 65537 ?tx4 true]
-    [?tx4 :db/txInstant ?ms4 ?tx4 true]}
-  :test/expected-tempids
-  {"t1" 65536
-   "t2" 65537}}
-
- {:test/label "conflicting upserts fail"
-  :test/assertions
-  [[:db/add "t1" :db/ident :name/Ivan]
-   [:db/add "t1" :db/ident :name/Petr]]
-  :test/expected-transaction
-  nil
-  :test/expected-error-message
-  "Conflicting upsert"
-  ;; nil
-  }
-
- {:test/label "tempids in :db/retract that do upsert are fine"
-  :test/assertions
-  [[:db/add "t1" :db/ident :name/Ivan]
-   ;; This ref doesn't exist, so the assertion will be ignored.
-   [:db/retract "t1" :db.schema/attribute 103]]
-  :test/expected-transaction
-  #{[?tx5 :db/txInstant ?ms5 ?tx5 true]}
-  :test/expected-error-message
-  ""
-  :test/expected-tempids
-  {}}
-
- {:test/label "tempids in :db/retract that don't upsert fail"
-  :test/assertions
-  [[:db/retract "t1" :db/ident :name/Anonymous]]
-  :test/expected-transaction
-  nil
-  :test/expected-error-message
-  ""}
-
- ;; The upsert algorithm will first try to resolve "t1", fail, and then allocate both "t1" and "t2".
- {:test/label "multistep, both allocated"
-  :test/assertions
-  [[:db/add "t1" :db/ident :name/Josef]
-   [:db/add "t2" :db.schema/attribute "t1"]]
-  :test/expected-transaction
-  #{[65538 :db/ident :name/Josef ?tx6 true]
-    [65539 :db.schema/attribute 65538 ?tx6 true]
-    [?tx6 :db/txInstant ?ms6 ?tx6 true]}
-  :test/expected-error-message
-  ""
-  :test/expected-tempids
-  {"t1" 65538
-   "t2" 65539}}
-
- ;; Can't quite test this without more schema elements.
- ;; ;; This time, we can resolve both, but we have to try "t1", succeed, and then resolve "t2".
- ;; {:test/label "multistep, upserted allocated"
- ;;  :test/assertions
- ;;  [[:db/add "t1" :db/ident :name/Josef]
- ;;   [:db/add "t2" :db/ident "t1"]]
- ;;  :test/expected-transaction
- ;;  #{[65538 :db/ident :name/Josef]
- ;;    [65538 :db/ident :name/Karl]
- ;;    [?tx8 :db/txInstant ?ms8 ?tx8 true]}
- ;;  :test/expected-error-message
- ;;  ""
- ;;  :test/expected-tempids
- ;;  {"t1" 65538
- ;;   "t2" 65539}}
- ]