vvvvalvalval / datomock

Mocking and forking Datomic Peer connections in-memory.
MIT License
130 stars 6 forks source link

Changes that make log work in queries #8

Closed awkay closed 6 years ago

awkay commented 6 years ago

The log API has to be able to accept tx IDs, instants, or t. At the moment only t is supported. To accomplish this I had to record the tx id and instant into the log. I had to add code to find the txInstant entry, and then code to do the log range comparisons.

There is one complication: t and :db/id of the transaction are both longs, so detecting which is in use is impossible; however, it does seem highly unlikely that any given range or one or the other will overlap...e.g. t might by 6000 to 6012 while :db/id of the transaction goes from 138349867248 to some other really large number.

This final assumption may not be true, but I have no other ideas of how to handle it.

vvvvalvalval commented 6 years ago

@awkay Thanks for spotting the issue. I'm working on a fix; I can't merge this as is as it fails to address some edge cases (it's possible to craft txes which will fool the proposed resolution of :db/txInstant, and (min forkT endT) will fail if endT is a date).

awkay commented 6 years ago

Yeah, that's fine. I didn't have time to run it through paces beyond what I needed (running a (fork-lib datomock) (ironically) at the moment because I needed this to work). Overlooked the min, good catch.

awkay commented 6 years ago

I'd be interested in how Cognitect implements this...seems odd to pass two different kinds of longs to the same function ambiguously like that. I see no good way of ever being certain which you have, but perhaps the ranges can never overlap.

vvvvalvalval commented 6 years ago

Fixed in v0.2.2, see https://github.com/vvvvalvalval/datomock/commit/f80f2316a645db53200de61ae8474af538958e23#diff-0cd8874f1d83c00f19b48f0aebaca977R36 for the implementation.