[tx] Wrap positive integer entids in an Entid newtype #104

New issue

Open

opened 2020-08-06 16:54:58 +00:00 by gburd · 0 comments

gburd commented

2020-08-06 16:54:58 +00:00

(Migrated from github.com)

This is follow-up to https://github.com/mozilla/mentat/pull/171#discussion_r96086384 and #170. Right now, I'm representing positive integer entids as i64, since SQLite doesn't have an unsigned u64 type. It's an interesting challenge to really represent entids as an Entid newtype that is really u63. When I started, I didn't understand the rusqlite traits FromSql and ToSql; now that I do, this should be straight-forward. I'm filing this as a [good next bug], I guess, since it's not trivial. Use OrderedFloat for inspiration and remember that it never makes sense to do arithmetic on entids.

As a side benefit/hindrance while testing: this might allow Mentat to control the introduction of entids. The Clojure version accepted any integer as an entid, which was handy for testing but did not agree with Datomic. We might be able to make the only way to allocate an entid be via a tempid, which has good correctness properties: it'll be harder to make errors in transactions, and it'll be harder to pollute a datom store with disconnected components.

This is follow-up to https://github.com/mozilla/mentat/pull/171#discussion_r96086384 and #170. Right now, I'm representing positive integer entids as `i64`, since SQLite doesn't have an unsigned `u64` type. It's an interesting challenge to really represent entids as an `Entid` newtype that is really `u63`. When I started, I didn't understand the `rusqlite` traits `FromSql` and `ToSql`; now that I do, this should be straight-forward. I'm filing this as a _[good next bug]_, I guess, since it's not trivial. Use `OrderedFloat` for inspiration and remember that it never makes sense to do arithmetic on entids. As a side benefit/hindrance while testing: this might allow Mentat to _control the introduction of entids_. The Clojure version accepted any integer as an entid, which was handy for testing but did not agree with Datomic. We might be able to make the only way to allocate an entid be via a tempid, which has good correctness properties: it'll be harder to make errors in transactions, and it'll be harder to pollute a datom store with disconnected components.