Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

(tx) Replace :db/tx with (current-tx) transaction function and broade… #665

Merged
merged 2 commits into from
Apr 27, 2018

Conversation

ncalexan
Copy link
Member

…n support. (#664)

:db/tx (and Datomic's version, :datomic/tx) suffer from the same
ambiguities that [a v] lookup references do -- determining the type of
the result is context sensitive. (In this case, is :db/tx a reference
to the current transaction ID, or is it a valid keyword?) This commit
addresses the ambiguity by introducing a notion of a transaction
functions, and provides a little scaffolding for adding more (should
the need arise). I left the scaffolding in place rather than handling
just (current-tx) because I started trying to
implement (current-txInstant) as well, which is more difficult -- see
the comments.

It's worth noting that this approach generalizes more or less directly
to ?input variables, since those can be eagerly bound like the
implemented transaction function (current-tx).

@ncalexan ncalexan requested a review from rnewman April 26, 2018 22:57
// transaction functions that produce numeric values, we'll have to
// be more careful here, because a function that produces an integer
// value can be used where a double is expected. See also
// `SchemaTypeChecking.to_typed_value(...)`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The algebrizer function stuff has some coercions that might be useful to extract.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick response. I do expect there's some shared typechecking stuff between query and parse; they're getting closer, not further apart :)

///
/// A natural next step might be to expose the current transaction instant `(current-txInstant)`,
/// but that's more difficult: the transaction itself can set the transaction instant (with some
/// restrictions), so the transaction function must be late-binding. Right now, that's difficult to
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you can work around that by simply defining (current-txInstant) as the timestamp that the transactor would use by default.

That would allow callers to do fun things like impose a timestamp but keep the transactor's timestamp around in a separate property, or correct clock drift, or whatever.

You could define the transactor's behavior as having a default

[:db/add (current-tx) :db/txInstant (current-txInstant)]

… which 'feels' better if you call it (current-instant) or (transact-instant).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, in fact, that (transaction-tx) and (transaction-instant) might be better than any reference to "current", since current might be reasonably interpreted to mean "the latest transaction committed" or "the timestamp when I built this data structure". I find that (current-txInstant) blurs that considerably.

As for allowing an eager (current-txInstant) while allowing to set :db/txInstant... it just seems so prone to error. Follow-up, for sure.

…oaden support. (mozilla#664)

:db/tx (and Datomic's version, :datomic/tx) suffer from the same
ambiguities that [a v] lookup references do -- determining the type of
the result is context sensitive.  (In this case, is :db/tx a reference
to the current transaction ID, or is it a valid keyword?)  This commit
addresses the ambiguity by introducing a notion of a transaction
functions, and provides a little scaffolding for adding more (should
the need arise).  I left the scaffolding in place rather than handling
just (transaction-tx) because I started trying to
implement (transaction-instant) as well, which is more difficult --
see the comments.

It's worth noting that this approach generalizes more or less directly
to ?input variables, since those can be eagerly bound like the
implemented transaction function (transaction-tx).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants