[tx] Accept [a v] notation for lookup refs in entity position #152

New issue

Open

opened 2020-08-06 16:55:48 +00:00 by gburd · 0 comments

gburd commented

2020-08-06 16:55:48 +00:00

(Migrated from github.com)

This is follow-up tracking https://github.com/mozilla/mentat/pull/382#pullrequestreview-28255925. In #382, I accept only (lookup-ref a v) for lookup-refs in both entity and value position (e and v in a [:db/add e _ v] assertion). That's contrary to Datomic, which accepts embedded vectors like [a v] in both locations, treating ambiguity in some ad-hoc way. (See the discussion in #183 for notes on this ambiguity.)

This ticket tracks accepting an embedded vector [a v] in the entity position only. We suspect that the majority of lookup refs are entity refs in this form, and it's both shorter to type and handy to agree with Datomic.

To implement this, you'll need to:

verify that entid_or_lookup_ref_or_temp_id is still used only to parse entity positions;
add a vector_lookup_ref function accepting [a v] around 8adb6d97fd/tx-parser/src/lib.rs (L72);
use your new function around 8adb6d97fd/tx-parser/src/lib.rs (L88);
add tests demonstrating you can used nested vectors in entity position and that you can use nested vectors in value position -- but they're not interpreted as lookup refs. This latter may already be tested by the existing tests!

This is follow-up tracking https://github.com/mozilla/mentat/pull/382#pullrequestreview-28255925. In #382, I accept only `(lookup-ref a v)` for lookup-refs in both entity and value position (`e` and `v` in a `[:db/add e _ v]` assertion). That's contrary to Datomic, which accepts embedded vectors like `[a v]` in both locations, treating ambiguity in some ad-hoc way. (See the discussion in #183 for notes on this ambiguity.) This ticket tracks accepting an embedded vector `[a v]` in the entity position only. We suspect that the majority of lookup refs are entity refs in this form, and it's both shorter to type and handy to agree with Datomic. To implement this, you'll need to: 1) verify that `entid_or_lookup_ref_or_temp_id` is still used only to parse entity positions; 2) add a `vector_lookup_ref` function accepting `[a v]` around https://github.com/mozilla/mentat/blob/8adb6d97fd55b262d7d89708db86eb37f0e5ab0d/tx-parser/src/lib.rs#L72; 3) use your new function around https://github.com/mozilla/mentat/blob/8adb6d97fd55b262d7d89708db86eb37f0e5ab0d/tx-parser/src/lib.rs#L88; 4) add tests demonstrating you can used nested vectors in entity position and that you can use nested vectors in value position -- but they're not interpreted as lookup refs. This latter may already be tested by the existing tests!