Add constraint name to foreign key/check constraint errors

[aaronrenner]

Is your feature request related to a problem? Please describe.

I'm running into an issue where Ecto isn't able to nicely handle cockroach's constraint errors (foreign_key_violation, unique_violation, exclusion_violation and check_violation ) because cockroach's constraint errors do not include a constraint_name error field as part of the error response. This behavior was added in postgres 9.3 under a feature titled "Provide clients with constraint violation details as separate fields".

Describe the solution you'd like

It would be great if cockroach could add the failing constraint name to these errors, so they can be used by higher level application code. The field id should be n as described in 53.8. Error and Notice Message Fields of the postgres docs. This field is also know as PG_DIAG_CONSTRAINT_NAME in the PQresultErrorField docs.

It would also be nice if the error message text could match that of postgres, but I understand if there's reasons that it's been changed.

Additional context
Here are the differences between the postgres and cockroach foreign key error responses, as parsed by postgrex.

# Postgres error fields (as parsed by postgrex)
%Postgrex.Error{
  connection_id: 70007,
  message: nil,
  postgres: %{
    code: :foreign_key_violation,
    constraint: "users_account_id_fkey",
    detail: "Key (account_id)=(q) is not present in table \"accounts\".",
    file: "ri_triggers.c",
    line: "2776",
    message: "insert or update on table \"users\" violates foreign key constraint \"users_account_id_fkey\"",
    pg_code: "23503",
    routine: "ri_ReportViolation",
    schema: "public",
    severity: "ERROR",
    table: "users",
    unknown: "ERROR"
  },
  query: nil
}

# Cockroach error fields (as parsed by postgresx)
%Postgrex.Error{
  connection_id: nil,
  message: nil,
  postgres: %{
    code: :foreign_key_violation,
    file: "sql/sqlbase/fk.go",
    line: "359",
    message: "foreign key violation: value ['k'] not found in accounts@primary [id] (txn=8c87c634-cf34-4e69-8bf3-ea6758e65a82)",
    pg_code: "23503",
    routine: "runCheck",
    severity: "ERROR"
  },
  query: nil
}

Here are the differences in the error messages between cockroach and postgres:

• Postgres: insert or update on table "users" violates foreign key constraint "users_account_id_fkey"
• Cockroach: foreign key violation: value ['k'] not found in accounts@primary [id] (txn=8c87c634-cf34-4e69-8bf3-ea6758e65a82)

Issue tracking ecto compatibility: #33441

Psycopg2 issue where they were working to support additional PG_DIAG_*_NAME fields like PG_DIAG_CONSTRAINT_NAME: psycopg/psycopg2#149

[jordanlewis]

Thanks for the report. So, to be clear, Ecto uses the constraint_name notice messages to do something important, and it fails to do so because CockroachDB doesn't send those messages, right?

Does Ecto also parse the error message itself?

[aaronrenner]

That is correct.

Ecto first attempts to grab the constraint name out of the ErrorResponse's constraint name field, and then falls back to parsing the error message itself in the case it's talking to Postgres 9.2 and earlier.

To support Ecto, all we really need is the constraint name field on the ErrorResponse.

[aaronrenner]

Also, if you'd really like to see what Ecto is doing, here's where Ecto does its pattern matching on the error responses: https://github.com/elixir-ecto/ecto_sql/blob/1a6c92a001912fe3df7046fc09dbb1611241b548/lib/ecto/adapters/postgres/connection.ex#L18-L58. The first 4 function heads are where it matches on the constraint field and the next 4 function heads are where they fall back to parsing the error message.

[jordanlewis]

@RaduBerinde is this done, or easier now?

[RaduBerinde]

I was not aware of this, I'll do some research.

[knz]

My understanding of the work to do here is like this:

• add the new field to pgerror.Error
• ensure it's carried over through pgwire
• to ease troubleshooting, extend pkg/cli/error.go to also display the value of the new field
• populate the pgerror.Error field during constraint check errors

The tricky bit is that we (usually) forget about the names when we do physical planning. But that's where Radu has an edge.

[RaduBerinde]

Thanks, makes sense. I guess I would add something similar to WithSeverity / GetSeverity to be able to attach it to errors and then have it make its way into pgerror.Error.