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

Update dolt_verify_constraints() stored procedure to support all constraint types #2093

Merged
merged 1 commit into from Apr 3, 2024

Conversation

fulghum
Copy link
Contributor

@fulghum fulghum commented Apr 2, 2024

Removing the wording that says only FK constraints are supported.

Also updated the result description – the docs stated we returned a count of the violations, but the code was actually just returning 0 for no violations and 1 for violations. This would be nice to fix so that we return the actual violation count, but I think we should look at migrating dolt constraints verify to the stored procedure first, since it will likely require other result changes, and then we can batch them together in one minor version update.

Related to: dolthub/dolt#7679

Copy link
Sponsor Contributor

@timsehn timsehn left a comment

Choose a reason for hiding this comment

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

What does this mean?

DOLT_VERIFY_CONSTRAINTS by default does not detect constraints for row changes

@fulghum
Copy link
Contributor Author

fulghum commented Apr 2, 2024

What does this mean?

DOLT_VERIFY_CONSTRAINTS by default does not detect constraints for row changes

The rest of that sentence, on the next line (hidden by the diff on GitHub), is "that have been previously committed." – so it's just saying, that by default, dolt_verify_constraints() only detected changes in the working set, but you can use the --all flag to verify all data, even rows that have already been committed.

@timsehn
Copy link
Sponsor Contributor

timsehn commented Apr 2, 2024

Cool LGTM.

@fulghum fulghum merged commit f2737cc into gitbook-dev Apr 3, 2024
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.

None yet

2 participants