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

sql: add constraint name to constraint error #55660

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

alex-berger
Copy link
Contributor

@alex-berger alex-berger commented Oct 17, 2020

Add constraint name to error for unique constraint, check constraintand foreign key constraint violations. Those constraint names are then propagated over the PostgreSQL wire protocol and will show up for example in JDBC exceptions (org.postgresql.util.PSQLException#getServerErrorMessage().getConstraint()). This commit improves PostgreSQL compatibility.

Release note: Improve PosgreSQL wire protocol compatibility by adding constraint name to sql errors.

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Oct 17, 2020

CLA assistant check
All committers have signed the CLA.

@blathers-crl
Copy link

blathers-crl bot commented Oct 17, 2020

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

  • A person you worked with closely on this PR.
  • The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
  • Join our community slack channel and ask on #contributors.
  • Try find someone else from here.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner labels Oct 17, 2020
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@blathers-crl
Copy link

blathers-crl bot commented Oct 17, 2020

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

  • A person you worked with closely on this PR.
  • The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
  • Join our community slack channel and ask on #contributors.
  • Try find someone else from here.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

Thanks! :lgtm: I just have a couple of nits.

nit: the commit message should have a title, a body, and a release note (basically what you have in the PR description).

nit: the title of the commit message (and the PR) should start with the name of the package that has changed and the words should be in lower-case, i.e. we want something like sql: add constraint name to errors.

nit: for the release note, we should specify a category, I think in this case it would be Release note (sql change): ....

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @alex-berger)


pkg/sql/row/errors.go, line 98 at r1 (raw file):

	}

	error := pgerror.Newf(pgcode.UniqueViolation,

super nit: it might be good to not declare a local variable and return the error right away for consistency with other changes.

Add constraint name to error for unique constraint, check constraint, and foreign key constraint violations. Those constraint names are then propagated over the PostgreSQL wire protocol and will show up for example in JDBC exceptions (org.postgresql.util.PSQLException#getServerErrorMessage().getConstraint()). This commit improves PostgreSQL compatibility of CockroachDB error messages.

Release note (sql change): Add constraint name to constraint errors, to increase wire level PostgreSQL compatibility.
@blathers-crl
Copy link

blathers-crl bot commented Oct 20, 2020

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@yuzefovich yuzefovich changed the title Add constraint name to errors sql: add constraint name to constraint error Oct 20, 2020
@yuzefovich
Copy link
Member

Thanks!

bors r=yuzefovich

@craig
Copy link
Contributor

craig bot commented Oct 20, 2020

Build succeeded:

@craig craig bot merged commit ae78ca7 into cockroachdb:master Oct 20, 2020
@alex-berger alex-berger deleted the feature/constraint_name branch October 21, 2020 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants