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

Don't allow violation of cardinality-one restrictions within a single transact.#533

Closed
rnewman wants to merge 2 commits into
masterfrom
rnewman/tx-cardinality-one-existing
Closed

Don't allow violation of cardinality-one restrictions within a single transact.#533
rnewman wants to merge 2 commits into
masterfrom
rnewman/tx-cardinality-one-existing

Conversation

@rnewman
Copy link
Copy Markdown
Collaborator

@rnewman rnewman commented Jan 23, 2018

This requires maintaining a unique index so that we can INSERT OR REPLACE. The last value will win. We could also do INSERT OR IGNORE, check the number of changed rows, and throw.

This only works if the lookup ref succeeds. If it doesn't, then we're into #532.

Thoughts, @ncalexan?

@rnewman rnewman self-assigned this Jan 23, 2018
@rnewman rnewman requested a review from ncalexan January 23, 2018 02:56
Copy link
Copy Markdown
Member

@ncalexan ncalexan left a comment

Choose a reason for hiding this comment

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

In general, I think this is great. Perhaps a little expensive, but clear.

I have no strong opinion on OR REPLACE vs OR IGNORE. Last one wins -- i.e., transaction is processed in the order it is presented -- certainly seems to be most clear.

I am a little concerned that it violates Datomic's set-oriented model, but I am comforted by the fact that Datomic doesn't allow, for example, a schema change in the first datom transacted and then the use of the new schema fragment in subsequent datoms transacted. (In the same transaction.)

Does this not address the map notation bug you originally started from? If it does not, OK; if it does, please include a map notation test, since there is some hairy interleaving of the transactor stages that should be exercised.

Comment thread db/src/db.rs
[:db/add "foo" :test/unique "x"]
]"#);

// You can try to assert two values for the same entity and attribute,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this agree with Datomic? With DataScript?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Datomic raises an error:

":db.error/datoms-conflict Two datoms in the same transaction conflict\n{:d1 [17592186045418 :test/one 123 13194139534313 true],\n :d2 [17592186045418 :test/one 124 13194139534313 true]}\n"

@rnewman
Copy link
Copy Markdown
Collaborator Author

rnewman commented Jan 23, 2018

We can't easily detect duplicates within a tx boundary: we can use INSERT and have an opaque error about uniqueness constraints being violated, which we wrap to Could not insert non-fts one statements into temporary search table!; use INSERT OR IGNORE and silently take the first value; or INSERT OR REPLACE and silently take the last (based on some permutation of the input order, I assume).

I think I'd prefer the opaque error, which we can improve in the future. I'll file that now.

@rnewman
Copy link
Copy Markdown
Collaborator Author

rnewman commented Jan 23, 2018

ef9f2d9 with a map notation test.

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