Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move unique constraint checking until after optimistic insertion #2037

Merged
merged 2 commits into from
Dec 6, 2024

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Dec 4, 2024

Description of Changes

Move unique constraint checking until after the row has been inserted and roll back if there was a violation.
We need to do this to enable insertion via BSATN as we no longer will have a PV.
After insertion, we can instead project RowRefs to the index types.
This should also facilitate moving indices out of tables.

Testing

Proptests are amended to reflect new order.

@Centril Centril requested a review from gefjon December 4, 2024 12:17
@gefjon
Copy link
Contributor

gefjon commented Dec 4, 2024

What's the deal with all the test failures?

@Centril Centril force-pushed the centril/late-constraint-checking branch from 1eebec1 to 1d4d126 Compare December 4, 2024 18:24
@Centril
Copy link
Contributor Author

Centril commented Dec 4, 2024

What's the deal with all the test failures?

Seems one file ended up in the stash that shouldn't have.

@gefjon
Copy link
Contributor

gefjon commented Dec 4, 2024

Can you bring in latest master to fix the "Internal Tests" job?

@Centril Centril force-pushed the centril/late-constraint-checking branch from 1d4d126 to 57fd564 Compare December 4, 2024 18:28
@Centril
Copy link
Contributor Author

Centril commented Dec 4, 2024

Can you bring in latest master to fix the "Internal Tests" job?

Yep, done.

@Centril Centril force-pushed the centril/late-constraint-checking branch from 57fd564 to f45b027 Compare December 4, 2024 18:57
Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

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

AFK and not gonna do a full review RN; will follow up in the morning. Just don't want to forget this.

@Centril Centril force-pushed the centril/late-constraint-checking branch 3 times, most recently from d15faf7 to 459601f Compare December 5, 2024 13:11
@Centril Centril force-pushed the centril/late-constraint-checking branch from 459601f to 46094e0 Compare December 5, 2024 14:52
Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

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

Minor nits, mostly comment-ey. Code looks good, and is correct AFAICT.

crates/table/src/btree_index.rs Outdated Show resolved Hide resolved
crates/table/src/btree_index.rs Show resolved Hide resolved
crates/table/src/btree_index/multimap.rs Outdated Show resolved Hide resolved
@Centril Centril force-pushed the centril/late-constraint-checking branch from 11391fe to c3f304a Compare December 6, 2024 11:49
@Centril Centril added this pull request to the merge queue Dec 6, 2024
Merged via the queue into master with commit 96a3871 Dec 6, 2024
8 checks passed
@Centril Centril deleted the centril/late-constraint-checking branch December 6, 2024 12:36
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