sfackler / rust-postgres

Native PostgreSQL driver for the Rust programming language
Apache License 2.0
3.49k stars 443 forks source link

copy_out with BINARY on ltree type has a single hex char prefix #960

Closed auyer closed 2 years ago

auyer commented 2 years ago

The return value using copy_out with BINARY has this single hex char prefix for ltree type: image which does not happen in copy_out with CSV or query_raw image

Notice here: https://github.com/sfu-db/connector-x/pull/382#issuecomment-1289777545 Thanks to @wangxiaoying

sfackler commented 2 years ago

That is part of the binary encoding of the ltree type. https://github.com/sfackler/rust-postgres/blob/master/postgres-protocol/src/types/mod.rs#L1063-L1080

auyer commented 2 years ago

I see. Could there be a Postgres Type Ltree in the postgres::types::Type module to deal with this ? Or would this be out of the scope of this crate ? Thanks!

sfackler commented 2 years ago

It can already be converted to and from Rust strings.

wangxiaoying commented 2 years ago

Please correct me if I'm wrong. Base on my understanding, BinaryCopyOutRow::try_get will invoke ltree_from_sql for ltree type, which should already removed the version character in the beginning based on the code you pointed above. Which means the result we got from try_get should not contain this prefix. However, the above result (e.g. \u{1}A.B.C.D, printed from the &str result we get from the try_get method of BinaryCopyOutRow) still has this version character.

Is there anything that I'm missing? The code for getting the printed value val can be found here: https://github.com/sfu-db/connector-x/blob/main/connectorx/src/sources/postgres/mod.rs#L419

sfackler commented 2 years ago

I can't reproduce that behavior. This test:

#[test]
fn binary_copy_ltree() {
    let mut client = Client::connect("host=localhost port=5433 user=postgres", NoTls).unwrap();

    client
        .batch_execute(
            "
        CREATE TEMPORARY TABLE foo (a ltree);

        INSERT INTO foo (a) VALUES ('A.B.C.D'), ('A.B.E'), ('A');
    ",
        )
        .unwrap();

    let stmt = client.prepare("SELECT a FROM foo").unwrap();

    let reader = client.copy_out("COPY foo TO STDOUT BINARY").unwrap();
    let mut it = BinaryCopyOutIter::new(reader, &[stmt.columns()[0].type_().clone()]);

    while let Some(row) = it.next().unwrap() {
        println!("{:?}", row.get::<Option<&str>>(0));
    }
}

Prints this when run:

running 1 test
Some("A.B.C.D")
Some("A.B.E")
Some("A")
test test::binary_copy_ltree ... ok
wangxiaoying commented 2 years ago

Hi @sfackler, I think the reason we got the error is because the type param we put for BinaryCopyOutIter::new is TEXT instead of Other(Other { name: "ltree", oid: 16905, kind: Simple, schema: "public" }).

Thanks a lot for the help! Please feel free to close the issue. @auyer