Postgresql error context (where) information is missing, please consider including 'W' context error field.

[staninprague]

Feature Flags

postgresql

Problem Description

Error reporting with diesel::result::Error::DatabaseError is missing context information for postgresql. This makes it harder to troubleshoot where database error happens.

What are you trying to accomplish?

E.g. I have a before delete trigger that calls a function that has more fields in INSERT INTO (fields...) than SELECT that follows.

Error presented by many clients when I try to delete the row that would trigger the trigger/function is:

SQL Error [42601]: ERROR: INSERT has more target columns than expressions
  Where: PL/pgSQL function fn_insert_point_log_on_deleted_collection() line 6 at SQL statement

What is the actual output?

Diesel most detailed presentation of the error is:

"error":"INSERT has more target columns than expressions, kind: __Unknown, details: None, hint: None, table: None, column: None, constraint: None"

Only message field is non-empty in this case via Diesel:

INSERT has more target columns than expressions

kind, details, hint, table, column, constraint name are all None.

What is the expected output?

One would expect diesel to provide that "where" bit of the information:

  PL/pgSQL function fn_insert_point_log_on_deleted_collection() line 6 at SQL statement

Per

enum ResultField {
    SqlState = 'C' as i32,
    MessagePrimary = 'M' as i32,
    MessageDetail = 'D' as i32,
    MessageHint = 'H' as i32,
    TableName = 't' as i32,
    ColumnName = 'c' as i32,
    ConstraintName = 'n' as i32,
}

And trait:

pub trait DatabaseErrorInformation {
fn message(&self) -> &str;
fn details(&self) -> Option<&str>;
fn hint(&self) -> Option<&str>;
fn table_name(&self) -> Option<&str>;
fn column_name(&self) -> Option<&str>;
fn constraint_name(&self) -> Option<&str>;
}

There is no way to convey that context/where information that is contained in https://github.com/postgres/postgres/blob/master/src/include/postgres_ext.h:

#define PG_DIAG_CONTEXT			'W'

It would be great if someone can decide if DatabaseErrorInformation deserves a new method like context() or if this context information can be added to the "details()" implementation of PgErrorInformation trait implementation:

impl DatabaseErrorInformation for PgErrorInformation {
    fn message(&self) -> &str {
        get_result_field(self.0.as_ptr(), ResultField::MessagePrimary)
            .unwrap_or_else(|| self.0.error_message())
    }

    fn details(&self) -> Option<&str> {
        get_result_field(self.0.as_ptr(), ResultField::MessageDetail)
    }

Adding this context information is something that would help the error logging in common, but I also think that it can improve situation with reporting migration errors? See #2378 - More verbose errors on diesel migrations?

I can help with implementation, but this is something that spans multiple database systems as pub trait DatabaseErrorInformation is generic and I'd need guidance on how to implement. I can imagine providing a PR with adding 'W' Context into ResultField and adding this to details of PgErrorInformation, but I'm not sure this would be the best approach.

Or I'm missing some easy way of getting this context information without extending anything?

[TaKO8Ki]

In #1572, wg tried to solve a similar problem. However, it has not been completed yet.

[staninprague]

In #1572, wg tried to solve a similar problem. However, it has not completed yet.

Suggested 'P' returns a string with a decimal integer of error position in the SQL.

Actually for the kind of error that is described in this issue, it returns as None.

I commented on the issue you mentioned. Hopefully the fact that 'P' contains a string with a decimal integer value may help to move with the awaiting PR/merge.

Though I'd still believe that adding context() based on 'W' is quite beneficial for troubleshooting the errors:

PG_DIAG_CONTEXT
An indication of the context in which the error occurred. Presently this includes a call stack traceback of active procedural language functions and internally-generated queries. The trace is one entry per line, most recent first.

May I prepare a PR with a context() method, similar to what was done in #1572 for 'P'?

[weiznich]

Opening a PR would certainly make it easier to discuss the design. Otherwise this seems like a meaningful addition for me.

[Mart-Bogdan]

SqlState also would be helpful as a separate getter.

Many DBMS supports error codes beyond string message

[Mart-Bogdan]

What about SqlState ? should it be addressed in another issue?

[weiznich]

@Mart-Bogdan That's completely unrelated to this issue. Feel free to write a proposal adding such an API in our discussion forum, but be sure to answer the questions outlined in our contribution guidelines.