Error querying PgBouncer

[jwilm]

PgBouncer seems to implement some, but not all, of the postgres protocol. Specifically, I was running into this error when trying to query it:

Db(DbError {
    severity: "ERROR",
    parsed_severity: None,
    code: ProtocolViolation,
    message: "unsupported pkt type: 80",
    detail: None,
    hint: None,
    position: None,
    where_: None,
    schema: None,
    table: None,
    column: None,
    datatype: None,
    constraint: None,
    file: None,
    line: None,
    routine: None
})

The error message is from PgBouncer internals (admin.c:1365). Basically, looks like the 'P' frame isn't handled. This is generated from postgres_protocol::messages::frontend::parse.

Any suggestions on how to resolve?

[sfackler]

It looks like PGBouncer doesn't support prepared statements. You can use batch_execute, but we don't currently expose a method that returns result rows.

The non-prepared statement API doesn't behave like the prepared statement API, so you won't be able to use the FromSql API - you'll just get a bunch of stings back.

[jwilm]

you'll just get a bunch of stings back

How can I do that? batch_execute returns Result<()>.

[sfackler]

This is the (private) method that batch_execute wraps: https://github.com/sfackler/rust-postgres/blob/master/postgres/src/lib.rs#L953

[jwilm]

That sounds like the right thing, but as you mentioned, it's private. How do you feel about exposing something like this through the public API?

[jwilm]

I tried this out -- simply changed the batch_execute API to pass through the Result from quick_query. Mostly works for my use case, but it doesn't seem to provide access to Row description (backend frame type T).

I'm thinking about adding a method similar to quick_query that would also provide Vec<Column> along with the string results. Actually, thinking about this a bit further, it seems that we would be able to provide Rows out of such a method given that we have the row description. The only blocker here seems to be the Arc<StatementInfo> on Rows. It looks to me like Rows and Row only ever use the columns field of StatementInfo. Maybe we could just stick that in an Arc instead of the whole StatementInfo?

To summarize this chain of thought, I'm proposing something like:

1. Add new public query interface like static_query(query: &str) -> Result<Rows>
2. To support Rows being returned without using prepared statement, change type of columns field on StatementInfo to Arc<Vec<Column>>, and have the Rows type store that instead of Arc<StatementInfo>.

[sfackler]

I don't think any interface returning Rows is going to work - this query interface returns tex -formatted values while FromSql etc work off of the binary value format.

[jwilm]

Ahh it appears you're right. I was confused since the example I was debugging specified text format instead of binary in the row description, but apparently simple queries are a special case where the result is always text (except for binary cursors).

In any case, it might still be helpful to return a Vec<Column> as part of the simple query API. For an interface, maybe something like this would make sense:

pub trait GenericConnection {
    // ...

    /// New method on GenericConnection; sends a simple "Query" frame
    ///
    /// Returns `Vec<TextRows>` since there may be multiple, semicolon delimited
    /// queries in the `query` string. From a protocol level, each query yields T, D, C
    /// (row description, data row(s), command completion), and finally a "ready
    /// for query" frame Z is returned.
    fn simple_query(&self, query: &str) -> Result<Vec<TextRows>>;
}

/// Result of one query sent via the `simple_query` API.
struct TextRows {
    /// Result of parsing Row description for query
    columns: Vec<Column>,
    /// Data rows returned in text format
    rows: Vec<RowData>,
}

/// One Row whose data is only available in string format
struct TextRow {
    columns: &[Column],
    raw: &RowData,
}

// Mirror existing Rows/Row APIs where it makes sense
impl IntoIterator for &TextRows {}
impl TextRows {
    fn columns(&self) -> &[Column];
    fn len(&self) -> usize;
    fn is_empty(&self) -> bool;
    fn get(&self, idx) -> TextRow;
    fn iter(&self) -> TextRowIter;
}

impl TextRow {
    fn columns(&self) -> &[Column];
    fn len(&self) -> usize;
    fn is_empty(&self) -> bool;
    fn get<I: RowIndex>(&self, idx: I) -> &str;
    fn get_opt<I: RowIndex>(&self, idx: I) -> Option<&str>;
}

[jwilm]

Started implementing this and realized it would be easier to store RowData rather than Vec<Option<String>>. Updated the proposal above to reflect that.

[sfackler]

That seems like a reasonable setup I think.

[jwilm]

Closing this since all relevant info is in #304.

[scirner22]

@sfackler I had to go this route to solve the pgbouncer prepared statement problem as well. Although, it seems like we should have a better solution. I was able to get it functional, but my personal complication came around having a column of bytea type. Using simple_query returns it back in a string literal form. As a workaround I had to set the bytea_output type to escape and parse the string response as ascii and convert to a byte array.

I'm far from having a good understanding of the postgres programming interface, but I was trying to do something as simple as conn.query("SELECT .., .. from <table> order by .."). I assume this library is creating a prepared statement and attempting to use it for future queries? That seems strange, but I can't think of any other reason why I would see problems with a command like that.

[sfackler]

Postgres has 2 query protocols - "simple" and "extended": https://www.postgresql.org/docs/12/protocol-flow.html#id-1.10.5.7.4. pgbouncer only handles the simple protocol properly due to how it routes requests, but that protocol doesn't support important features like query parameters.

[scirner22]

I understand that, but in my case I'm just trying to query with a string literal and I'm not using prepared statements at all.

conn.query("SELECT id, key, topic, data FROM kafka_messages ORDER BY id ASC", &[])?;

Something this simple now requires me to use the simple_query function and handle the complications around the new response type and value parsings.

Again, I'm no expert on the protocol itself, but I can use pgbouncer and jdbc totally transparently with parameterized queries that are more complex than my example above.

[codesections]

I have also encountered this issue, and have revised my code to use simple_query when used with pgbouncer. However, I notice that the documentation for simple_query states that it

should [not] be use[d] for any query which contains user-specified data, as they provided the functionality to safely embed that data in the request. Do not form statements via string concatenation and pass them to this method!

I'm a bit confused about how this interacts with the advice in this issue to use simple_query for all queries involving pgbouncer (since that will inevitably mean using simple_query with at least some user-provided data). Based on #575, it appears that rust-postgres does not currently provide a method for escaping user input when using simple_query; is that correct?

If so, what is the correct way to use pgbouncer for all Postgres queries (which, of course, includes queries with user-provided parameters)?

Thanks very much for any help and for rust-postgres as a whole!

[DGolubets]

I'm baffled with this situation in Rust.
Why can't we have something like prepareThreshold=0 in JDBC?

[jwilm]

@codesections The simple_query API should only needed when talking to the PgBouncer management API; it correctly proxies prepared statement destined for PostgreSQL but does not support prepared statements itself.