Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cannot convert &str to Postgres reg* types #1041

Open
dmfay opened this issue Jun 3, 2023 · 12 comments
Open

cannot convert &str to Postgres reg* types #1041

dmfay opened this issue Jun 3, 2023 · 12 comments

Comments

@dmfay
Copy link

dmfay commented Jun 3, 2023

A query select $1::regclass (textual representation of a table oid) passed an &str tablename yields the error:

error serializing parameter 0: cannot convert between the Rust type `&str` and the Postgres type `regclass`

It's easy to work around (select $1::text::regclass) so I'm raising this issue to document that, but it'd be nice to get the conversions set up for regclass, regtype, regproc, regnamespace, etc!

@sfackler
Copy link
Owner

sfackler commented Jun 3, 2023

Those types are aliases of oid, so they would be converted to and from u32, not str. Seems reasonable to add support though.

@trungda
Copy link
Contributor

trungda commented Aug 13, 2023

@sfackler could you help advise how to add support for these reg types? I would be happy to contribute here.

These types are weird. They can be casted to both text and oid. I think in the syntax "select $1::regclass",
"$1" should be a string?

@sfackler
Copy link
Owner

You'd just need to change these two lines to support the other appropriate types:
https://github.com/sfackler/rust-postgres/blob/master/postgres-types/src/lib.rs#L729
https://github.com/sfackler/rust-postgres/blob/master/postgres-types/src/lib.rs#L1158

@trungda
Copy link
Contributor

trungda commented Aug 14, 2023

You'd just need to change these two lines to support the other appropriate types: https://github.com/sfackler/rust-postgres/blob/master/postgres-types/src/lib.rs#L729 https://github.com/sfackler/rust-postgres/blob/master/postgres-types/src/lib.rs#L1158

I looked a bit into this by writing a test:

#[tokio::test]
async fn regclass() {
    let conn = connect("user=postgres").await;
    // Create a table foo
    conn.execute("CREATE TABLE IF NOT EXISTS foo (c1 INT);", &[])
        .await
        .unwrap();
    test_type("regclass", &[(Some("foo".to_owned()), "'foo'")]).await;
}

I might be missing something but I think we should add support to cast from &str to REGCLASS (not u32)?

Thank you!

@sfackler
Copy link
Owner

How would you propose to do that?

@trungda
Copy link
Contributor

trungda commented Aug 14, 2023

I am thinking that we would have to modify FromSql and ToSql implementations of &str to accept these new types. E.g.,:

impl<'a> FromSql<'a> for &'a str {
    fn from_sql(ty: &Type, raw: &'a [u8]) -> Result<&'a str, Box<dyn Error + Sync + Send>> {
        match *ty {
            ref ty if ty.name() == "ltree" => types::ltree_from_sql(raw),
            ref ty if ty.name() == "lquery" => types::lquery_from_sql(raw),
            ref ty if ty.name() == "ltxtquery" => types::ltxtquery_from_sql(raw),
            ref ty if ty.name() == "regclass" => types::regclass_from_sql(raw), // something like this.
            _ => {
                types::text_from_sql(raw)
            },
        }
    }

and the relevant bits. What do you think?

@sfackler
Copy link
Owner

The wire format for regclass is a 32 bit unsigned integer.

@trungda
Copy link
Contributor

trungda commented Aug 14, 2023

Ah right! Let me look into how libpq handles the reg-type family.

@trungda
Copy link
Contributor

trungda commented Aug 15, 2023

So I think that libpq doesn't do anything magical (I was confused with psql). If we pass SELECT $1::regclass where $1 is a string, libpq would bind $1 to a string; and if we pass SELECT $1::regclass where $1 is a number, libpq would bind a number value to the $1 placeholder. In both cases, the response would be an oid.
Only in psql, the tool would do an extra query to look up system catalog to convert from the oid to the table name (this is where I was confused as I thought that libpq would somehow convert the oid to the table name magically).

Now, back to our quest, I think we should implement FromSQL to serialize the u32 response from server to regclass. As for ToSql, what can we do to support SELECT $1::regclass where $1 can be either a string or a number? Or should we straight-up not support that syntax?

@sfackler
Copy link
Owner

The existing implementations work fairly directly with the postgres binary format of the equivalent type.

@trungda
Copy link
Contributor

trungda commented Aug 20, 2023

Great! So I guess as you suggested, we can just add

simple_from!(u32, oid_from_sql, REGCLASS);
simple_to!(u32, oid_to_sql, REGCLASS);

for regclass and the reg-types?

@sfackler
Copy link
Owner

There can only be one impl per type, so you'd need to modify the existing lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants