Entity should return nil for numeric eids that don't correspond to entities

[lynaghk]

Ref: https://github.com/tonsky/datascript/issues/447

I'm not sure about the fastest way to check that an entity exists in the database. I used (empty? (db/-search db [e])) here, but presumably there's some allocation overhead from creating an iterator.

Perhaps there's a cheaper way to do it (maybe seeking on the index directly and checking for an eid match?) I'm open to suggestions and am happy to take a second look later with profiling if desired.

[lynaghk]

Seems like there's a bit more work to be done based on the failing JavaScript test here: https://github.com/tonsky/datascript/blob/cb06ac762e2f99d30bdc8fb09b49709eac5e5999/test/js/tests.js#L289

caused by the 101 not resolving to anything. I.e., there is no {:db/id 101} entity created when the numeric 101 is added as a child reference here: https://github.com/tonsky/datascript/blob/cb06ac762e2f99d30bdc8fb09b49709eac5e5999/test/js/tests.js#L283

There's a similar CLJ test, but it passes because all entities are defined at the toplevel https://github.com/tonsky/datascript/blob/cb06ac762e2f99d30bdc8fb09b49709eac5e5999/test/datascript/test/entity.cljc#L49-L52

Note there's one failing JavaScript test here: https://github.com/tonsky/datascript/blob/cb06ac762e2f99d30bdc8fb09b49709eac5e5999/test/js/tests.js#L289

caused by the 101 not resolving to anything. I.e., there is no {:db/id 101} entity created when the numeric 101 is added as a child reference here: https://github.com/tonsky/datascript/blob/cb06ac762e2f99d30bdc8fb09b49709eac5e5999/test/js/tests.js#L283

There's a similar CLJ test, but it passes because all entities are defined at the toplevel https://github.com/tonsky/datascript/blob/cb06ac762e2f99d30bdc8fb09b49709eac5e5999/test/datascript/test/entity.cljc#L49-L52

What's the desired behavior for transactions with references to positive numeric IDs that don't correspond to entities? For maps it's pretty clear the entities should be created. But for numeric IDs, I'm not sure. (Especially not sure for positive numbers, since tempid always returns negative ones.)

[tonsky]

I'm not sure about the fastest way to check that an entity exists in the database.

Interesting question! I’d go with something like

(= eid (-> (-seek-datoms db :eavt eid) first :e))

Might be a bit faster because it only does one binary search for left boundary. For some reason I thought there was some obvious easy way to check for entity, but I guess there isn’t. Maybe that’s why Datomic does not check eid, too.

What's the desired behavior for transactions with references to positive numeric IDs that don't correspond to entities? For maps it's pretty clear the entities should be created. But for numeric IDs, I'm not sure. (Especially not sure for positive numbers, since tempid always returns negative ones.)

Interesting question! I guess we want to throw, but then it’ll be a breaking change... Also should make sure that forward-referencing something still works with tempids (clauses are resolved one-by-one, and tempids replaced with allocated new eids, hopefully won’t be a problem) like this:

[[:db/add -1 :friend -2]
 [:db/add -2 :friend -1]]

Hopefully it’ll be ok to throw in this case though (both 1 and 2 do not exist yet). If you want this, use tempids:

[[:db/add 1 :friend 2]
 [:db/add 2 :friend 1]]

[lynaghk]

I had a go at throwing when positive IDs which don't correspond to entities are used in transactions, but this turned out to be pretty tricky --- it seems like a fair bit of the code right now relies on the behavior that positive IDs will pass through entid and entid-strict even if they don't correspond to entities that exist.

To keep this PR small, I just fixed the public API entity behavior and the (arguably broken) JS tests which relied on the above behavior.

Let's revisit the question of positive IDs in a separate issue / PR, since I suspect it'll require reworking a lot of internals.

[tonsky]

Thanks! Merged and published as 1.4.2. Let’s hope world will survive :)