Unknown OID issue with Pgbouncer

[wingedpig]

v4.1.2, but I think also v3. With newer versions of pgbouncer. I try to run telegraf, which uses the pgx library to talk with pgbouncer, I get unknown oid: 1700, name: total_xact_count from the query SHOW STATS. If I log the OIDs passed to DataTypeForOID, they are 25 and then 1700. It appears the oidToDataType map is not initialized before this.

I'm not familiar with the pgx code, this was just gleaned through printf debugging. Please let me know if I can provide any additional information.

[jackc]

I'm not sure but I suspect that error is coming from somewhere else in the stack.

I don't see any where that would produce the text of the message you are seeing. In addition in v4 there should never be a time when oidToDataType is not initialized with the built-in PostgreSQL types -- including 1700 numeric.

pgx includes tests for pgbouncer and while not exhaustive they do successfully exercise the basic query path.

...

I am unfamiliar with telegraf, but it looks like they are still using v3 of pgx -- https://github.com/influxdata/telegraf/blob/master/plugins/inputs/postgresql/service.go.

Are you sure you are running v4?

[wingedpig]

The error message is being produced on line 160 of query.go. I thought I had upgraded to v4 in my testing, but it appears I'm still testing with v3.6.

The pgbouncer plugin of telegraf does 2 queries, SHOW STATS and then SHOW POOLS. SHOW STATS fails, but the plugin never checks for the error code. The second query runs fine. So the output is just the result of the second query, with no error message produced, even though there was an error from the first query.

If I just comment out the first query, the second query still runs fine. So it seems to me that it's just an issue with the SHOW STATS query.

[jackc]

You are right the oidToDataType is not initialized for numeric. However, this appears to be because telegraf is purposely skipping the pg_type introspection. See https://github.com/influxdata/telegraf/blob/master/plugins/inputs/postgresql/service.go#L118 . CustomConnInfo overrides the standard connection procedure and manually registers something for int8. I'm not sure why that is being done ... maybe they want to avoid the introspection queries...

Beyond that, I'm pretty sure that the int8 registration is wrong. It looks like they are registering the oid type to handle int8. Those are different size integers. I suppose it happens to usually work because it is also using the text protocol. A 32 vs 64 bit integer wouldn't cause a problem unless the value was greater than 32 bits.

Anyway, I suspect that adding to that CustomConnInfo function to register pgtype.Numeric for pgtype.NumericOID (1700) would solve the original problem.

[wingedpig]

That was the issue. I registered pgtype.NumericOID. I also had to convert the results from strings to ints. And it worked. Thank you for investigating this, especially since it involved going through someone else's code. I will work up a pull request for telegraf.

[danielnelson]

CustomConnInfo overrides the standard connection procedure and manually registers something for int8. I'm not sure why that is being done ... maybe they want to avoid the introspection queries...

Hi @jackc, I'm one of the Telegraf maintainers. We're not especially concerned about introspection queries, I'm unsure the reason for skipping it other that it appears to be required for pgbouncer. If I remove the CustomConnInfo I get:

ERROR: invalid command 'select t.oid,
        case when nsp.nspname in ('pg_catalog', 'public') then t.typname
                else nsp.nspname||'.'||t.typname
        end
from pg_type t
left join pg_type base_type on t.typelem=base_type.oid
left join pg_class base_cls ON base_type.typrelid = base_cls.oid
left join pg_namespace nsp on t.typnamespace=nsp.oid
where (
          t.typtype in('b', 'p', 'r', 'e')
          and (base_type.oid is null or base_type.typtype in('b', 'p', 'r'))
        )', use SHOW HELP; (SQLSTATE 08P01)

If you can help set me straight on how we could have this handled by introspection on connection it would be much appreciated.

Otherwise, I'll make the change for the Int8, if I'm understanding correctly this change would take care of the potential overflow:

info.RegisterDataType(pgtype.DataType{
	Value: &pgtype.Int8{},
	Name:  "int8OID",
	OID:   pgtype.Int8OID,
})

[jackc]

I'm not familiar enough with PgBouncer to know if it is necessary to skip the introspection queries or not.

But it turned out that introspection was unneeded for pgx anyway. All supported types have a fixed OID except hstore. In v4 we replaced introspection with a hard coded mapping.

It's perfectly reasonable to just make the change for Int8. If you need any additional types you can check out https://github.com/jackc/pgtype/blob/282b7936a2cd6528fd3c4cdab4232a514ca54adb/pgtype.go#L178 which is what v4 uses for its hard-coded registrations. Should be fairly trivially to adapt.