Expose PQresultErrorField

[sgrove]

This should close #23, and let us build much more robust error handling (using regexps right now).

Let me know if you'd like anything changed here, would love to get a new release.

An example of what this looks like afterwards (in Reason syntax):

type errorFieldName =
  | Severity
  | SeverityNonlocalized
  | SqlState
  | MessagePrimary
  | MessageDetail
  | MessageHint
  | StatementPosition
  | InternalPosition
  | InternalQuery
  | Context
  | SchemaName
  | TableName
  | ColumnName
  | DataTypeName
  | ConstraintName
  | SourceFile
  | SourceLine
  | SourceFunction;

let errorField res (fieldName: errorFieldName) =>
  res#error_field (
    switch fieldName {
    | Severity => 'S'
    | SeverityNonlocalized => 'V'
    | SqlState => 'C'
    | MessagePrimary => 'M'
    | MessageDetail => 'D'
    | MessageHint => 'H'
    | StatementPosition => 'P'
    | InternalPosition => 'p'
    | InternalQuery => 'q'
    | Context => 'W'
    | SchemaName => 's'
    | TableName => 't'
    | ColumnName => 'c'
    | DataTypeName => 'd'
    | ConstraintName => 'n'
    | SourceFile => 'F'
    | SourceLine => 'L'
    | SourceFunction => 'R'
    }
  );

/* Example usage */
...
let res = OP.fetch_single_result conn;
let status = res#status;
print_endline (errorField res TableName)

[mmottl]

Thanks for contributing! Here are some suggestions for improvement:

I'd recommend adding the type definition of error field names (e.g. SEVERITY, etc.) to the bindings instead of passing characters to the C-stubs. This is safer for the user, because they cannot pass illegal characters. It's usually easier to compare representations with the C-API if we keep the tag capitalization and use _ between words within the tag. It also indicates that these tags are coming from an underlying C-library. Most OCaml users also prefer underscores instead of capitals in type names, e.g. error_field_name instead of errorFieldName. You may even consider moving the type definition into submodule Error_field_name as type t (not strictly necessary, the bindings are already a little inconsistent in that respect anyway).

The conversion between the tag and the PostgreSQL code should happen within the C-stubs to guarantee stability across protocol changes. You can take a look at oid_tbl in the C-stubs on how to do deal with conversion tables between C- and OCaml-tags.

Finally, you may want to use Int_val to convert from (tagged) OCaml integers to C integers instead of shifting explicitly. This should be more future-safe. The table approach above for tag conversions is preferable though.

[sgrove]

Thanks for such detailed suggestions, it helped quite a bit!

I've pushed up a new commit that goes down the path you outlined, and I'm happy to clean it up. I tried to surface some of the error codes, etc. (ultimately, we want to be able to easily handle cases based off of e.g. uniqueness violation in a stable way), but it may be overboard.

Here's an example of what it looks like in usage right now:

let res = OP.fetch_single_result conn
print_endline ("Error type PG_DIAG_SEVERITY: " ^ (res#error_field PG_DIAG_SEVERITY))
print_endline ("Error type PG_DIAG_SQLSTATE: " ^ (res#error_field PG_DIAG_SQLSTATE))
print_endline ("\tCondition: " ^
     (match res#error_field_condition_name with
      | UNIQUE_VIOLATION  -> "Unique-constraint violated!"
      | _ -> "Some other error case"))
print_endline ("Error type PG_DIAG_SOURCE_FILE: " ^ (res#error_field PG_DIAG_SOURCE_FILE))
print_endline ("Error type PG_DIAG_SOURCE_LINE: " ^ (res#error_field PG_DIAG_SOURCE_LINE))
print_endline ("Error type PG_DIAG_SOURCE_FUNCTION: " ^ (res#error_field PG_DIAG_SOURCE_FUNCTION))

And example of the info we can extract on an error:

Error type PG_DIAG_SEVERITY: ERROR
Error type PG_DIAG_SEVERITY_NONLOCALIZED: ERROR
Error type PG_DIAG_SQLSTATE: 23505
Error type PG_DIAG_MESSAGE_PRIMARY: duplicate key value violates unique constraint "apps_name_key"
Error type PG_DIAG_MESSAGE_DETAIL: Key (name)=(example) already exists.
Error code in psql: 23505
    Condition: Unique-constraint violated!
Error type PG_DIAG_MESSAGE_HINT:
Error type PG_DIAG_STATEMENT_POSITION:
Error type PG_DIAG_INTERNAL_POSITION:
Error type PG_DIAG_INTERNAL_QUERY:
Error type PG_DIAG_CONTEXT:
Error type PG_DIAG_SCHEMA_NAME: public
Error type PG_DIAG_TABLE_NAME: app
Error type PG_DIAG_COLUMN_NAME:
Error type PG_DIAG_DATATYPE_NAME:
Error type PG_DIAG_CONSTRAINT_NAME: apps_name_key
Error type PG_DIAG_SOURCE_FILE: nbtinsert.c
Error type PG_DIAG_SOURCE_LINE: 433
Error type PG_DIAG_SOURCE_FUNCTION: _bt_check_unique

What would you like to see changed so that we can get this merged and published?

[mmottl]

Thanks for the improvements! I have made some more changes in this branch:

https://github.com/mmottl/postgresql-ocaml/tree/better-error-handling

The error field names and error codes, including their conversion functions, now reside in separate files. Especially the size of the error code module would otherwise have cluttered the main file too much. I've also removed the intermediate integer representation of error field names. It didn't seem to help much on the OCaml side and made the bindings more complex.

Please let me know whether the current API is sufficient for your purposes and whether the implementation changes still work for you. I'll make a new release then.

[sgrove]

This looks great, thank you very much for everything.

[mmottl]

Thanks for the patches, the new distribution should be online soon.