Skip to content

Conversation

ash14
Copy link
Contributor

@ash14 ash14 commented Dec 18, 2020

PR to address #378. Not sure how dirty the implementation is for this ... but tests pass so 🤷

Cheers.

@tonsky
Copy link
Owner

tonsky commented Dec 18, 2020

Thank you for the bug report and the PR. This is an interesting problem :)

I feel that tempid resolution should not happen in resolve-upserts, but somewhere earlier, around

(and (ref? db a) (tempid? v))
(if-some [resolved (get tempids v)]
(let [report' (update report ::value-tempids assoc resolved v)]
(recur report' (cons [op e a resolved] entities)))
(let [resolved (next-eid db)
report' (-> report
(allocate-eid v resolved)
(update ::value-tempids assoc resolved v))]
(recur report' es)))

Basically, same code should run, but for all tuple values, not just for a single v.

Also, checking for being a tuple is better not by coll?, but by going to schema

(defn #?@(:clj [^Boolean tuple?]
:cljs [^boolean tuple?]) [db attr]
(is-attr? db attr :db.type/tuple))

@ash14 ash14 marked this pull request as draft December 21, 2020 09:33
@ash14
Copy link
Contributor Author

ash14 commented Dec 21, 2020

I've updated the test case - the entity to upsert now comes first which breaks the approach I was taking with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants