Make DbError fields public

All of them are directly exposed via accessors and the internal
representation is never realistically going to change. A single private
field allows for expansion in the future.

Author: sfackler

src/error.rs

@@ -17,168 +17,123 @@ include!(concat!(env!("OUT_DIR"), "/sqlstate.rs"));
 /// A Postgres error or notice.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct DbError {
-    severity: String,
-    code: SqlState,
-    message: String,
-    detail: Option<String>,
-    hint: Option<String>,
-    position: Option<ErrorPosition>,
-    where_: Option<String>,
-    schema: Option<String>,
-    table: Option<String>,
-    column: Option<String>,
-    datatype: Option<String>,
-    constraint: Option<String>,
-    file: String,
-    line: u32,
-    routine: String,
-}
-
-impl DbErrorNew for DbError {
-    fn new_raw(fields: Vec<(u8, String)>) -> result::Result<DbError, ()> {
-        let mut map: HashMap<_, _> = fields.into_iter().collect();
-        Ok(DbError {
-            severity: try!(map.remove(&b'S').ok_or(())),
-            code: SqlState::from_code(try!(map.remove(&b'C').ok_or(()))),
-            message: try!(map.remove(&b'M').ok_or(())),
-            detail: map.remove(&b'D'),
-            hint: map.remove(&b'H'),
-            position: match map.remove(&b'P') {
-                Some(pos) => Some(ErrorPosition::Normal(try!(pos.parse().map_err(|_| ())))),
-                None => {
-                    match map.remove(&b'p') {
-                        Some(pos) => {
-                            Some(ErrorPosition::Internal {
-                                position: try!(pos.parse().map_err(|_| ())),
-                                query: try!(map.remove(&b'q').ok_or(())),
-                            })
-                        }
-                        None => None,
-                    }
-                }
-            },
-            where_: map.remove(&b'W'),
-            schema: map.remove(&b's'),
-            table: map.remove(&b't'),
-            column: map.remove(&b'c'),
-            datatype: map.remove(&b'd'),
-            constraint: map.remove(&b'n'),
-            file: try!(map.remove(&b'F').ok_or(())),
-            line: try!(map.remove(&b'L').and_then(|l| l.parse().ok()).ok_or(())),
-            routine: try!(map.remove(&b'R').ok_or(())),
-        })
-    }
-
-    fn new_connect<T>(fields: Vec<(u8, String)>) -> result::Result<T, ConnectError> {
-        match DbError::new_raw(fields) {
-            Ok(err) => Err(ConnectError::DbError(Box::new(err))),
-            Err(()) => Err(ConnectError::IoError(::bad_response())),
-        }
-    }
-
-    fn new<T>(fields: Vec<(u8, String)>) -> Result<T> {
-        match DbError::new_raw(fields) {
-            Ok(err) => Err(Error::DbError(Box::new(err))),
-            Err(()) => Err(Error::IoError(::bad_response())),
-        }
-    }
-}
-
-impl DbError {
     /// The field contents are ERROR, FATAL, or PANIC (in an error message),
     /// or WARNING, NOTICE, DEBUG, INFO, or LOG (in a notice message), or a
     /// localized translation of one of these.
-    pub fn severity(&self) -> &str {
-        &self.severity
-    }
+    pub severity: String,
 
     /// The SQLSTATE code for the error.
-    pub fn code(&self) -> &SqlState {
-        &self.code
-    }
+    pub code: SqlState,
 
     /// The primary human-readable error message. This should be accurate but
     /// terse (typically one line).
-    pub fn message(&self) -> &str {
-        &self.message
-    }
+    pub message: String,
 
     /// An optional secondary error message carrying more detail about the
     /// problem. Might run to multiple lines.
-    pub fn detail(&self) -> Option<&str> {
-        self.detail.as_ref().map(|s| &**s)
-    }
+    pub detail: Option<String>,
 
     /// An optional suggestion what to do about the problem. This is intended
     /// to differ from Detail in that it offers advice (potentially
     /// inappropriate) rather than hard facts. Might run to multiple lines.
-    pub fn hint(&self) -> Option<&str> {
-        self.hint.as_ref().map(|s| &**s)
-    }
+    pub hint: Option<String>,
 
     /// An optional error cursor position into either the original query string
     /// or an internally generated query.
-    pub fn position(&self) -> Option<&ErrorPosition> {
-        self.position.as_ref()
-    }
+    pub position: Option<ErrorPosition>,
 
     /// 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.
-    pub fn where_(&self) -> Option<&str> {
-        self.where_.as_ref().map(|s| &**s)
-    }
+    pub where_: Option<String>,
 
     /// If the error was associated with a specific database object, the name
     /// of the schema containing that object, if any. (PostgreSQL 9.3+)
-    pub fn schema(&self) -> Option<&str> {
-        self.schema.as_ref().map(|s| &**s)
-    }
+    pub schema: Option<String>,
 
     /// If the error was associated with a specific table, the name of the
     /// table. (Refer to the schema name field for the name of the table's
     /// schema.) (PostgreSQL 9.3+)
-    pub fn table(&self) -> Option<&str> {
-        self.table.as_ref().map(|s| &**s)
-    }
+    pub table: Option<String>,
 
     /// If the error was associated with a specific table column, the name of
     /// the column. (Refer to the schema and table name fields to identify the
     /// table.) (PostgreSQL 9.3+)
-    pub fn column(&self) -> Option<&str> {
-        self.column.as_ref().map(|s| &**s)
-    }
+    pub column: Option<String>,
 
     /// If the error was associated with a specific data type, the name of the
     /// data type. (Refer to the schema name field for the name of the data
     /// type's schema.) (PostgreSQL 9.3+)
-    pub fn datatype(&self) -> Option<&str> {
-        self.datatype.as_ref().map(|s| &**s)
-    }
+    pub datatype: Option<String>,
 
     /// If the error was associated with a specific constraint, the name of the
     /// constraint. Refer to fields listed above for the associated table or
     /// domain. (For this purpose, indexes are treated as constraints, even if
     /// they weren't created with constraint syntax.) (PostgreSQL 9.3+)
-    pub fn constraint(&self) -> Option<&str> {
-        self.constraint.as_ref().map(|s| &**s)
-    }
+    pub constraint: Option<String>,
 
     /// The file name of the source-code location where the error was reported.
-    pub fn file(&self) -> &str {
-        &self.file
-    }
+    pub file: String,
 
     /// The line number of the source-code location where the error was
     /// reported.
-    pub fn line(&self) -> u32 {
-        self.line
-    }
+    pub line: u32,
 
     /// The name of the source-code routine reporting the error.
-    pub fn routine(&self) -> &str {
-        &self.routine
+    pub routine: String,
+
+    _p: (),
+}
+
+impl DbErrorNew for DbError {
+    fn new_raw(fields: Vec<(u8, String)>) -> result::Result<DbError, ()> {
+        let mut map: HashMap<_, _> = fields.into_iter().collect();
+        Ok(DbError {
+            severity: try!(map.remove(&b'S').ok_or(())),
+            code: SqlState::from_code(try!(map.remove(&b'C').ok_or(()))),
+            message: try!(map.remove(&b'M').ok_or(())),
+            detail: map.remove(&b'D'),
+            hint: map.remove(&b'H'),
+            position: match map.remove(&b'P') {
+                Some(pos) => Some(ErrorPosition::Normal(try!(pos.parse().map_err(|_| ())))),
+                None => {
+                    match map.remove(&b'p') {
+                        Some(pos) => {
+                            Some(ErrorPosition::Internal {
+                                position: try!(pos.parse().map_err(|_| ())),
+                                query: try!(map.remove(&b'q').ok_or(())),
+                            })
+                        }
+                        None => None,
+                    }
+                }
+            },
+            where_: map.remove(&b'W'),
+            schema: map.remove(&b's'),
+            table: map.remove(&b't'),
+            column: map.remove(&b'c'),
+            datatype: map.remove(&b'd'),
+            constraint: map.remove(&b'n'),
+            file: try!(map.remove(&b'F').ok_or(())),
+            line: try!(map.remove(&b'L').and_then(|l| l.parse().ok()).ok_or(())),
+            routine: try!(map.remove(&b'R').ok_or(())),
+            _p: (),
+        })
+    }
+
+    fn new_connect<T>(fields: Vec<(u8, String)>) -> result::Result<T, ConnectError> {
+        match DbError::new_raw(fields) {
+            Ok(err) => Err(ConnectError::DbError(Box::new(err))),
+            Err(()) => Err(ConnectError::IoError(::bad_response())),
+        }
+    }
+
+    fn new<T>(fields: Vec<(u8, String)>) -> Result<T> {
+        match DbError::new_raw(fields) {
+            Ok(err) => Err(Error::DbError(Box::new(err))),
+            Err(()) => Err(Error::IoError(::bad_response())),
+        }
     }
 }
 

src/lib.rs

@@ -229,7 +229,7 @@ pub struct LoggingNoticeHandler;
 
 impl HandleNotice for LoggingNoticeHandler {
     fn handle_notice(&mut self, notice: DbError) {
-        info!("{}: {}", notice.severity(), notice.message());
+        info!("{}: {}", notice.severity, notice.message);
     }
 }
 
@@ -474,8 +474,8 @@ impl InnerConnection {
                                 WHERE t.oid = $1") {
             Ok(..) => return Ok(()),
             Err(Error::IoError(e)) => return Err(ConnectError::IoError(e)),
-    // Range types weren't added until Postgres 9.2, so pg_range may not exist
-            Err(Error::DbError(ref e)) if e.code() == &SqlState::UndefinedTable => {}
+            // Range types weren't added until Postgres 9.2, so pg_range may not exist
+            Err(Error::DbError(ref e)) if e.code == SqlState::UndefinedTable => {}
             Err(Error::DbError(e)) => return Err(ConnectError::DbError(e)),
             _ => unreachable!(),
         }

tests/test.rs

@@ -56,7 +56,7 @@ fn test_prepare_err() {
     let conn = or_panic!(Connection::connect("postgres://postgres@localhost", SslMode::None));
     let stmt = conn.prepare("invalid sql database");
     match stmt {
-        Err(Error::DbError(ref e)) if e.code() == &SyntaxError && e.position() == Some(&Normal(1)) => {}
+        Err(Error::DbError(ref e)) if e.code == SyntaxError && e.position == Some(Normal(1)) => {}
         Err(e) => panic!("Unexpected result {:?}", e),
         _ => panic!("Unexpected result"),
     }
@@ -65,7 +65,7 @@ fn test_prepare_err() {
 #[test]
 fn test_unknown_database() {
     match Connection::connect("postgres://postgres@localhost/asdf", SslMode::None) {
-        Err(ConnectError::DbError(ref e)) if e.code() == &InvalidCatalogName => {}
+        Err(ConnectError::DbError(ref e)) if e.code == InvalidCatalogName => {}
         Err(resp) => panic!("Unexpected result {:?}", resp),
         _ => panic!("Unexpected result"),
     }
@@ -339,7 +339,7 @@ fn test_batch_execute_error() {
 
     let stmt = conn.prepare("SELECT * FROM foo ORDER BY id");
     match stmt {
-        Err(Error::DbError(ref e)) if e.code() == &UndefinedTable => {}
+        Err(Error::DbError(ref e)) if e.code == UndefinedTable => {}
         Err(e) => panic!("unexpected error {:?}", e),
         _ => panic!("unexpected success"),
     }
@@ -382,7 +382,7 @@ FROM (SELECT gs.i
       ORDER BY gs.i
       LIMIT 2) ss"));
     match stmt.query(&[]) {
-        Err(Error::DbError(ref e)) if e.code() == &CardinalityViolation => {}
+        Err(Error::DbError(ref e)) if e.code == CardinalityViolation => {}
         Err(err) => panic!("Unexpected error {:?}", err),
         Ok(_) => panic!("Expected failure"),
     };
@@ -532,7 +532,7 @@ fn test_custom_notice_handler() {
 
     impl HandleNotice for Handler {
         fn handle_notice(&mut self, notice: DbError) {
-            assert_eq!("note", notice.message());
+            assert_eq!("note", notice.message);
             unsafe { count += 1; }
         }
     }
@@ -648,7 +648,7 @@ fn test_cancel_query() {
     });
 
     match conn.execute("SELECT pg_sleep(10)", &[]) {
-        Err(Error::DbError(ref e)) if e.code() == &QueryCanceled => {}
+        Err(Error::DbError(ref e)) if e.code == QueryCanceled => {}
         Err(res) => panic!("Unexpected result {:?}", res),
         _ => panic!("Unexpected result"),
     }
@@ -708,7 +708,7 @@ fn test_plaintext_pass_no_pass() {
 fn test_plaintext_pass_wrong_pass() {
     let ret = Connection::connect("postgres://pass_user:asdf@localhost/postgres", SslMode::None);
     match ret {
-        Err(ConnectError::DbError(ref e)) if e.code() == &InvalidPassword => {}
+        Err(ConnectError::DbError(ref e)) if e.code == InvalidPassword => {}
         Err(err) => panic!("Unexpected error {:?}", err),
         _ => panic!("Expected error")
     }
@@ -733,7 +733,7 @@ fn test_md5_pass_no_pass() {
 fn test_md5_pass_wrong_pass() {
     let ret = Connection::connect("postgres://md5_user:asdf@localhost/postgres", SslMode::None);
     match ret {
-        Err(ConnectError::DbError(ref e)) if e.code() == &InvalidPassword => {}
+        Err(ConnectError::DbError(ref e)) if e.code == InvalidPassword => {}
         Err(err) => panic!("Unexpected error {:?}", err),
         _ => panic!("Expected error")
     }
@@ -745,12 +745,12 @@ fn test_execute_copy_from_err() {
     or_panic!(conn.execute("CREATE TEMPORARY TABLE foo (id INT)", &[]));
     let stmt = or_panic!(conn.prepare("COPY foo (id) FROM STDIN"));
     match stmt.execute(&[]) {
-        Err(Error::DbError(ref err)) if err.message().contains("COPY") => {}
+        Err(Error::DbError(ref err)) if err.message.contains("COPY") => {}
         Err(err) => panic!("Unexpected error {:?}", err),
         _ => panic!("Expected error"),
     }
     match stmt.query(&[]) {
-        Err(Error::DbError(ref err)) if err.message().contains("COPY") => {}
+        Err(Error::DbError(ref err)) if err.message.contains("COPY") => {}
         Err(err) => panic!("Unexpected error {:?}", err),
         _ => panic!("Expected error"),
     };
@@ -761,7 +761,7 @@ fn test_batch_execute_copy_from_err() {
     let conn = or_panic!(Connection::connect("postgres://postgres@localhost", SslMode::None));
     or_panic!(conn.execute("CREATE TEMPORARY TABLE foo (id INT)", &[]));
     match conn.batch_execute("COPY foo (id) FROM STDIN") {
-        Err(Error::DbError(ref err)) if err.message().contains("COPY") => {}
+        Err(Error::DbError(ref err)) if err.message.contains("COPY") => {}
         Err(err) => panic!("Unexpected error {:?}", err),
         _ => panic!("Expected error"),
     }