sfackler / rust-postgres Goto Github PK

I'm pasting the first README example into editor and trying to compile it, but getting this error:

➜  helioapp  cargo build
       Fresh phf_mac v0.0.0 (https://github.com/sfackler/rust-phf#ref=eeec95c7da4dd4fe29813c1a18722b9314320436)
       Fresh phf v0.0.0 (https://github.com/sfackler/rust-phf#ref=eeec95c7da4dd4fe29813c1a18722b9314320436)
       Fresh openssl v0.0.0 (https://github.com/sfackler/rust-openssl#ref=8696b091402f67a655c3b924c118392cb67540a3)
       Fresh rust-postgres v0.0.0 (https://github.com/sfackler/rust-postgres)
   Compiling helioapp v0.0.1 (file:/Users/valentin/Projects/personal/helioapp)
src/helioapp.rs:40:17: 40:24 error: cannot index a value of type `postgres::PostgresRow<'_>`
src/helioapp.rs:40             id: row[0u],
                                   ^~~~~~~
src/helioapp.rs:41:19: 41:26 error: cannot index a value of type `postgres::PostgresRow<'_>`
src/helioapp.rs:41             name: row[1u],
                                     ^~~~~~~
src/helioapp.rs:42:27: 42:34 error: cannot index a value of type `postgres::PostgresRow<'_>`
src/helioapp.rs:42             time_created: row[2u],
                                             ^~~~~~~
src/helioapp.rs:43:19: 43:26 error: cannot index a value of type `postgres::PostgresRow<'_>`
src/helioapp.rs:43             data: row[3u]
                                     ^~~~~~~
error: aborting due to 4 previous errors
Could not compile `helioapp`.

What am I doing wrong?

My version:
➜ helioapp rustc --version
rustc 0.12.0-pre-nightly (2224edcfe 2014-07-24 00:26:14 +0000)

It looks like drop() doesn't cond.signal() to wake the cond.wait().

Is there a reason { name: date, oid: 1082 } is not included? I realize a timestamp is more precise, but (at the very least) existing databases may use date.

I tried to add support for it at https://github.com/mikedilger/rust-postgres/commit/7f4637956f65c9086705c444ba93d30c4f593177 but I'm not sure that's right. When I try using it, it triggers a RefCell already borrowed error (#34 or rust-lang/rust#13246)

I don't think I've seen this in a driver before; what motivated it?

It seems a little weird to me because it's part of the type system but depends on an external system (postgres's transaction state).

I'm just starting now to learn Rust in order to compare it to Go and Dart primarily. I'm interested partly in testing database packages.

Attempting to compile rust-postgres resulted in the following error (win7):

c:\Users\Brian\rust-dev\dev\rust-postgres>rustc --lib lib.rs
lib.rs:922:8: 924:54 error: too many arguments to fmt!. found 3, expected 1
lib.rs:922 assert!(self.param_types.len() == params.len(),
lib.rs:923 "Expected {} parameters but found {}",
lib.rs:924 self.param_types.len(), params.len());

I presume that this is because I am not using the cutting-edge version.

Would it be feasible for you to freeze a release to keep it in step with the official release?

It's currently 1-indexed for consistency with query parameters, but it might make more sense to 0-index it for consistency with the rest of Rust.

On master, everything returns a Result<T, ~str>. On native, error "handling" is task failure. Neither of these options are particularly great.

The Result strategy imposes an annoying burden on users who don't care about handling SQL syntax errors in the common case. ~str is also not the appropriate type for errors, but that's more easily fixable.

On the other hand, users do sometimes want to do reasonable things with failure, especially when connecting and potentially when querying (e.g. if you wanted to rewrite psql).

One option is to have something like

impl PostgresConnection {
    pub fn connect(url: &Url) -> PostgresConnection {
        match PostgresConnection::try_connect(url) {
            Ok(conn) => conn,
            Err(err) => fail!(err)
        }
    }

    pub fn try_connect(url: &Url) -> Result<PostgresConnection, PostgresConnectionError> {
        ....
    }
}

and so on for all of the relevant methods. I think this is probably the most workable solution.

Whatever the solution is, the error object type should be robust. We should be able to parse the SQLSTATE value of the ErrorResult to get things like NotNullViolation vs ForeignKeyViolation.

Right now we're using the derived ToStr implementation.

I'm trying to support a user-defined enum type as follows. In postgres:

CREATE TYPE "field_type_t" AS ENUM ('text','tick','pick','date');

In rust:

#[deriving(Show,Encodable)]
pub enum FieldType {
    FieldTypeText,
    FieldTypeTick,
    FieldTypePick,
    FieldTypeDate,
}

impl FromSql for FieldType {
    #[allow(unused_variable)]
    fn from_sql(ty: &PostgresType, raw: &Option<Vec<u8>>) -> PostgresResult<FieldType>
    {
        match *raw {
            Some(ref buf) => {
                let s = match String::from_utf8(buf.clone()) {
                    Err(_) => return Err(PgBadResponse),
                    Ok(s) => s,
                };
                match s.as_slice() {
                    "text" => return Ok(FieldTypeText),
                    "tick" => return Ok(FieldTypeTick),
                    "pick" => return Ok(FieldTypePick),
                    "date" => return Ok(FieldTypeDate),
                    _ => return Err(PgBadResponse),
                }
            },
            None => {
                return Err(PgBadResponse);
            },
        }
    }
}

impl ToSql for FieldType {
    #[allow(unused_variable)]
    fn to_sql(&self, ty: &PostgresType) -> PostgresResult<(Format, Option<Vec<u8>>)>
    {
        let bits: Vec<u8> = match *self {
            FieldTypeText => "text".to_string().into_bytes(),
            FieldTypeTick => "tick".to_string().into_bytes(),
            FieldTypePick => "pick".to_string().into_bytes(),
            FieldTypeDate => "date".to_string().into_bytes(),
        };
        Ok((Binary,Some(bits)))
    }
}

I encounter a problem with row.get():

task '' failed at 'error retrieving column size: The value was NULL', src/lib.rs:1434

(On master it is actually line 1418, I'm using a servo-url patched version)

extern crate postgres;

use postgres::{PostgresConnection, NoSsl};
use postgres::types::ToSql;

fn main() {
  let mut content : ~str = ~"";
  {
    let conn = PostgresConnection::connect("postgres://jdavis@localhost:5432/postgres", &NoSsl).unwrap();
    let stmt = conn.prepare("SELECT x FROM t").unwrap();
    for row in stmt.query([]).unwrap() {
      let val : i32 = row[1];
      content.push_str(format!("<li>{}</li>\n", val));
    }
  }

  println!("{}", content);
}

Causes a segfault on the rust-nightly package I installed today (rustc 0.10-pre).

Backtrace:

(gdb) bt
#0  0x00007ffff7a9235d in _int_free (av=0x7ffff7dd3740 <main_arena>, p=0x8743a0, have_lock=0) at malloc.c:3924
#1  0x0000000000511ccb in _$UP$$UP$reseeding..ReseedingRng$LT$StdRng$C$TaskRngReseeder$GT$::glue_drop.5898::hca63f8a2631aec80 ()
#2  0x00000000008736d8 in ?? ()
#3  0x00000000005c352b in rt::task::Task::run::closure.41636 ()
#4  0x00000000005c3432 in rt::task::Task::run::closure.41627 ()
#5  0x00000000005ce9fc in rust_try ()
#6  0x00000000005c3282 in rt::task::Task::run::h3233312d889b0104os9::v0.10.pre ()
#7  0x0000000000441ef4 in start::h6478af498774a94fzvd::v0.10.pre ()
#8  0x0000000000441ce4 in lang_start::hdd717b0e84ff646bTud::v0.10.pre ()
#9  0x000000000040664f in main ()

A handler based approach like set_notice_handler doesn't really make sense here, since the common response to a notification is probably going to be querying for updated data.

Something like libpq's PGnotices seems like the most straightforward solution, but both blocking and polling options both seem like they'd be useful.

Building rust-postgres fails with the following error:

$ make
rustc -O -Z debug-info -L submodules/rust-openssl/build/ -L submodules/rust-phf/build/ -L submodules/rust-phf/build/ --dep-info build/postgres.d \
        --out-dir build src/lib.rs
src/error.rs:18:54: 18:61 error: macro undefined: 'phf_map'
src/error.rs:18         static STATE_MAP: PhfMap<PostgresSqlState> = phf_map!(
                                                                     ^~~~~~~
error: aborting due to previous error
make: *** [build/libpostgres-e8d942e6-0.0.rlib] Segmentation fault: 11

https://gist.github.com/sfackler/4c4e93f896e881fae811

I like that solution there. It is reasonably readable, does compile-time checking, and matches the postgres constructors.

However, it was also suggested on IRC to use something more like:

range!('[' a, b ')')

which is also acceptable to me.

We at least want impls for extra::time::{Timespec, Tm}, the various flavors of [u8] and BigInt.

Fails to compile, with the error:

Build failed, waiting for other jobs to finish...
Could not compile `postgres`.

--- stderr
src/error.rs:451:9: 451:14 error: found `where` in ident position
src/error.rs:451     pub where: Option<String>,
                         ^~~~~
src/error.rs:501:13: 501:18 error: found `where` in ident position
src/error.rs:501             where: map.pop(&('W' as u8)),
                             ^~~~~
error: aborting due to 2 previous errors

The Cargo.toml file used was:

[package]

name = "nickel_postgres"
version = "0.0.0"
authors = [
    "bguiz"
]

[lib]

name = "nickel_postgres"
path = "src/nickel_postgres/lib.rs"
test = false

[dependencies.nickel]

git = "https://github.com/nickel-org/nickel.rs.git"

[dependencies.postgres]

git = "https://github.com/sfackler/rust-postgres.git"

Addtional info:

$ rustc -v && cargo -V
rustc 0.12.0-pre-nightly (d30001d04 2014-08-16 00:46:15 +0000)
cargo 0.0.1-pre-nightly (b272701 2014-08-14 23:16:18 -0700)

It seems dangerous to throw an uncatchable exception in a library like rust-postgres. In some cases it could be OK, like in a program with many tasks and the parent catches such failures, but rust-postgres could be called from anywhere.

I realize there's also a "get" method, which returns a Result instead.

It seems like the use of Result everywhere is safer, but it's just too much boilerplate in a lot of simple cases.

Thinking out loud, it would be really nice if we could do one of the following:

1. Have a nicer syntax for handling Results that doesn't interfere with the readability so much. Maybe a new keyword or sigil that can do something like try!, but would go at the end so it doesn't get in the way of reading the program flow.
2. Somehow do all of the translation and type resolution at the time you fetch the result, so that fetching individual fields later can't generate an error.

We'll currently fail if we see a message we didn't expect or get unparseable data for a row. There may be other cases as well. We should instead propagate the error upwards as we do for other IO errors.

I see two ways to go here. We can either have something like we have now:

let result = do conn.in_transaction |conn| {
    conn.prepare(...)
    ....
    Ok(thing) // or Err(thing)
}

and commit or roll back based on the return value, or do something like:

let trans = conn.transaction();
conn.prepare(...)
....
trans.commit(); // or...
trans.rollback(); // or...
let _ = trans; // implicit commit

The block syntax makes the scope of the in_transaction solution a bit clearer. The downside is that you are forced to return a Result at the end, and if you don't actually want a result out of the transaction, you have to manually type the return value (e.g. Ok::<(), ()>(())). It also adds an extra level of indentation which can be annoying.

The transaction object solution is kind of weird, and may be more of a C++-ism than proper Rustic conventions.

One solution to the return value issue is to have in_transaction not return a value, and have the block return an enum TransactionCompletion { Commit, Rollback }. This may not be super useable without once-fns though, since you'll have to "return" a value from the transaction by mutating a captured variable. Returning a (T, TransactionCompletion) pair is also a possibility.

Yet another alternative is a kind of merger of the block and object solutions:

let result = do conn.in_transaction |trans| {
    trans.prepare(...)
    ...
    trans.will_commit(); // or...
    trans.will_rollback();
    ...
    foo
}

Another thing to think about is if it's possible to prevent nested transactions at compilation time.

I could submit a one-character patch if needed ;-)

We currently pull all result rows into memory after a query, which is the right thing to do in the general case, but it is useful to process the result set in chunks.

src/error.rs:23:68: 24:29 error: failed to resolve. Maybe a missing `extern crate /// unable to connect to the database.`?
src/error.rs:23         static STATE_MAP: PhfMap<&'static str, PostgresSqlState> = phf_map!(
src/error.rs:24             $($code => $error),+
src/lib.rs:1:1: 1479:1 note: in expansion of phf_map!
src/error.rs:23:68: 25:11 note: expansion site
src/error.rs:13:1: 37:2 note: in expansion of make_errors!
src/error.rs:40:1: 357:2 note: expansion site
src/error.rs:23:68: 25:11 error: `/// unable to connect to the database.::/// A `PostgresCancelData` object can be created via` does not name a structure
src/error.rs:23         static STATE_MAP: PhfMap<&'static str, PostgresSqlState> = phf_map!(
src/error.rs:24             $($code => $error),+
src/error.rs:25         );
src/lib.rs:1:1: 1479:1 note: in expansion of phf_map!
src/error.rs:23:68: 25:11 note: expansion site
src/error.rs:13:1: 37:2 note: in expansion of make_errors!
src/error.rs:40:1: 357:2 note: expansion site
error: aborting due to 2 previous errors
Could not compile `postgres`.

I'm new to rust, but I'm willing to take a crack at fixing this. I just don't know where to start... :)

rustc 0.12.0-pre-nightly (0e784e168 2014-09-16 23:26:11 +0000)

Having to do something like

stmt.query([&10 as &ToSql, & &"foo" as &ToSql, &None::<uint> as &ToSql]);

Is a huge pain. This problem will be less painful once Rust supports cross-crate macro exports, and we can write a params! macro like

stmt.query(params!(&10, & &"foo", &None::<uint>))

Hopefully Rust will eventually implicitly cast and auto-borrow to trait objects so the syntax will end up as

stmt.query([10, &"foo", None::<uint>]);

Something to watch out for here is to make sure no-one's changing stuff like client-encoding or DateStyle.

I'm selecting over a million rows from Postgres, and first I'm seeing a long spike of activity at the DB, followed by a spike of activity on the client.
So it seems you request all data at once?

It seems that if you use an iterator in Python, it fetches batches of 2000 items.
This would be a neat feature to have. Now I have to wait for a long time, or implement my own LIMIT logic.

http://initd.org/psycopg/docs/cursor.html#cursor.itersize

Hey @sfackler! This is great. However for the sake of preventing effort duplication, have you seen @linuxfood's rustsqlite? It's another sqlite binding that I believe is a little further along than yours at the moment.

I find the name of the method "update" a little awkward because it's also the name of a SQL command. The following doesn't seem to read quite right to me:

conn.update("DELETE FROM foo WHERE id=7")

Is it too late to consider changing it to something like "exec" or "execute"?

I'm just getting started with Rust and wanted to try using a Postgres database, so I created a new project with cargo, and added rust-postgres as a dependency:

[dependencies.postgres]
git = "https://github.com/sfackler/rust-postgres.git"

The compilation then fails as follows:

$ cargo run --verbose
       Fresh phf v0.0.0 (https://github.com/sfackler/rust-phf#996edc69)
       Fresh phf_mac v0.0.0 (https://github.com/sfackler/rust-phf#996edc69)
       Fresh openssl v0.0.0 (https://github.com/sfackler/rust-openssl#c407530d)
   Compiling postgres v0.0.0 (https://github.com/sfackler/rust-postgres.git#abd60ef1)
     Running `rustc src/lib.rs --crate-name postgres --crate-type lib -g -C metadata=1731918feb625991 -C extra-filename=-1731918feb625991 --out-dir /Users/bastian/projects/rust-test/target/deps --dep-info /Users/bastian/projects/rust-test/target/.fingerprint/postgres-1731918feb625991/dep-lib-postgres -L /Users/bastian/projects/rust-test/target/deps -L /Users/bastian/projects/rust-test/target/deps --extern phf_mac=/Users/bastian/projects/rust-test/target/deps/libphf_mac-78b8694be35c3487.dylib --extern openssl=/Users/bastian/projects/rust-test/target/deps/libopenssl-fbe75530f7eda428.rlib --extern phf=/Users/bastian/projects/rust-test/target/deps/libphf-4fc01267f85abb4e.rlib`
src/error.rs:23:68: 27:21 error: failed to resolve. Maybe a missing `extern crate /// was successful or not. An error will only be returned if the driver was`?
src/error.rs:23         static STATE_MAP: PhfMap<&'static str, PostgresSqlState> = phf_map!(
src/error.rs:24             $($code => $error),+
src/error.rs:25         );
src/error.rs:26
src/error.rs:27         impl PostgresSqlState {
src/lib.rs:1:1: 1779:1 note: in expansion of phf_map!
src/error.rs:23:68: 25:11 note: expansion site
src/error.rs:13:1: 37:2 note: in expansion of make_errors!
src/error.rs:40:1: 356:2 note: expansion site
src/error.rs:23:68: 25:11 error: `/// was successful or not. An error will only be returned if the driver was::/// unable to connect to the database.` does not name a structure
src/error.rs:23         static STATE_MAP: PhfMap<&'static str, PostgresSqlState> = phf_map!(
src/error.rs:24             $($code => $error),+
src/error.rs:25         );
src/lib.rs:1:1: 1779:1 note: in expansion of phf_map!
src/error.rs:23:68: 25:11 note: expansion site
src/error.rs:13:1: 37:2 note: in expansion of make_errors!
src/error.rs:40:1: 356:2 note: expansion site
error: aborting due to 2 previous errors
Could not compile `postgres`.

Caused by:
  Process didn't exit successfully: `rustc src/lib.rs --crate-name postgres --crate-type lib -g -C metadata=1731918feb625991 -C extra-filename=-1731918feb625991 --out-dir /Users/bastian/projects/rust-test/target/deps --dep-info /Users/bastian/projects/rust-test/target/.fingerprint/postgres-1731918feb625991/dep-lib-postgres -L /Users/bastian/projects/rust-test/target/deps -L /Users/bastian/projects/rust-test/target/deps --extern phf_mac=/Users/bastian/projects/rust-test/target/deps/libphf_mac-78b8694be35c3487.dylib --extern openssl=/Users/bastian/projects/rust-test/target/deps/libopenssl-fbe75530f7eda428.rlib --extern phf=/Users/bastian/projects/rust-test/target/deps/libphf-4fc01267f85abb4e.rlib -Awarnings` (status=101)

Rustc is the latest HEAD:

$ rustc --version
rustc 0.12.0-dev

Right now we support trust, password and md5. Of the remaining methods, KerberosV5 is deprecated and probably not worth supporting and SCMCredential doesn't apply since we don't use Unix domain sockets. That leaves GSS and SSPI.

At least for primitive numeric types and byte arrays, this should help avoid a lot of allocations.

We should really be running Postgres both with SSL on and off to fully test the different SslModes.

Currently, we either ignore NoticeResponse messages or fail on them. This should probably be implemented as a macro that wraps match conn.read_message() {...} blocks with the proper NoticeResponse handling inserted.

Support converting a basic Rust value from and to a PostgresRow instance, so that the insertion and query api would be much cleaner.

For example, instead of

conn.execute("INSERT INTO person (name, time_created, data)
                    VALUES ($1, $2, $3)",
                 [&me.name as &ToSql, &me.time_created as &ToSql,
                  &me.data as &ToSql]);

you write

conn.execute("INSERT INTO person (name, time_created, data)
                    VALUES ($1, $2, $3)", me.toRow());

Not sure if it's worth, but it would be a nice concept to have something like that:

#[deriving(sql_create_table)]
#[sql(name="persons")]
struct Person {
    #[sql(pk)]
    id: i32,

    #[sql(limit = "255")]
    name: String,

    time_created: Timespec,

    #[sql(type="bytea")]
    data: Option<Vec<u8>>

    #![sql(unique(id, name))]
}

let sql = Person.sql_create_table();

It could also generate methods for insertion or update. Another idea:

Serial<T>, like an Option<T>, but shows if a record has already be inserted into the database or not. person.save(conn) will check it and either call insert or update. Used as in
struct Person { id: Serial<i32> }.

Most of the time you're not going to reuse a prepared statement. Update is trivial but query will take some thought since it will have to return an "augmented" result object that contains both the prepared statement and the normal result object so that the lifetimes work out.

It currently uses a match block which compiles down to an enormous chain of memcmps. If rust-lang/rust#4864 is ever implemented, it'd work well here.

These share .prepare, .execute, .batch_execute, and .transaction, it would be useful to allow either to be passed to functions that only require those methods.

Rust permits interior null bytes in strings but Postgres does not. Should we check for interior nulls or let the server complain?

First of all, please tell me if there is a better forum for discussion like this. Also, as a disclaimer, I am very new to rust.

A type is pretty useless unless it has conversions in both directions, right? So why not just make one trait for both directions?

Also, what is the model for binary versus text representations? Going in, it seems like the type can represent itself as either text or binary; but coming back, the type needs to either support both, or at least declare which one it does support.

This will become much more relevant when issue #7 is implemented. FromSql for float may have to be able to deal with numerous kinds of integer binary data if we want to be forgiving about conversions.

Blocked on rust-lang/rust#8953

Types defined by extensions don't work.

'error retrieving column login: Unexpected type PgUnknownType { name: citext, oid: 44398 }', src/lib.rs:1427

Commit 5c866ba notes: "In addition, an SSL encrypted connection can be used via Unix sockets". But, according to the PostgreSQL documentation, SSL communication is only possible with TCP/IP connections.

There doesn't seem to be any code that explicitly deals with SSL over Unix sockets, so there's nothing to remove. Still, it might be useful to prevent this combination in order to provide a more relevant error message.

ConnectionPool is basically a proof of concept. It's missing way too many features to be useful (it doesn't even check for dead connections!). It should either be improved or deleted. A real, robust implementation may be complex enough that it should be a separate project (e.g. c3p0).

I'm seeing pool.get_connection() return connections where the RefCell is WRITING (as opposed to UNUSED like it should be), so that the first use of the connection fails with RefCell<T> already borrowed. If I sleep(1) after getting the connection, then it becomes UNUSED, so it seems like a race condition.

This only happens when the client is cond.wait()ing in get_connection(), so that it gets a connection that was just previously used.

When trying to pattern match the postgres connection:

let conn = try!(PostgresConnection::connect("postgres://postgres@localhost", &NoSsl));

I get the following compile error:

<std macros>:2:59: 2:65 error: mismatched types: expected `()` but found `core::result::Result<<generic #14>,postgres::error::PostgresConnectError>` (expected () but found enum core::result::Result)
<std macros>:2     ($e:expr) => (match $e { Ok(e) => e, Err(e) => return Err(e) })

Not entirely sure why, because when looking at the source I see that the connection function returns the right types:

pub fn connect<T: IntoConnectParams>(params: T, ssl: &SslMode)
                                         -> Result<PostgresConnection,
                                                   PostgresConnectError> {
        InnerPostgresConnection::connect(params, ssl).map(|conn| {
            PostgresConnection { conn: RefCell::new(conn) }
        })
    }

test_cancel_query flickers very occasionally since there's a chance that the cancel request will hit an internal query generated by the driver. The right thing to do is probably to just propagate the error.

It occurred to me that we can provide absolute compile time SQL-injection safety by hiding prepare() and execute() behind syntax extensions.

Basically, instead of

let s = conn.prepare("SELECT id, name, time_created, data FROM person where id = $1").unwrap();
s.query([123])
conn.execute("INSERT INTO person (name, time_created, data) VALUES ($1, $2, $3)",
                       &[&me.name, &me.time_created, &me.data])

we have

let s = prepare!(conn, "SELECT id, name, time_created, data FROM person where id = $1").unwrap();
s.query([123]);
execute!(conn, "INSERT INTO person (name, time_created, data VALUES ($1, $2, $3)",
               &me.name, &me.time_created, &me.data)

Both prepare! and execute! will ensure at compile time that their first argument is a string literal (like format!, and similarly execute! will ensure that the number of arguments fits the number of parameters provided.

We can optionally extend the number of arguments protection to query as well, by making it a macro too: give the result of prepare! the type PostgresStatement<(String, String, String)> if the prepared query has three parameters, and ensure that query!(stmt, 1 , 2 , 3) indeed has three arguments after the first argument.

This sort of system would ensure that the programmer never uses string concatenation* for SQL, leaving sanitization to the library. As a bonus, no more sloppy array slices, macros have varargs!

*I have seen string concatenation being used in PDO prepared statements in PHP; you can drag a horse to the water but you can't make it drink ;) Rust seems to follow the "don't trust the programmer" principle, and this design seems to nicely follow it