search

will / crystal-pg

a postgres driver for crystal
BSD 3-Clause "New" or "Revised" License
463 stars 78 forks source link

add register decoder with oid #180

Closed mamantoha closed 5 years ago

mamantoha commented 5 years ago

Fixes https://github.com/imdrasil/jennifer.cr/issues/258

https://github.com/imdrasil/jennifer.cr/blob/d8b665d287a046fabcec5bf12003e689412db23f/src/jennifer/adapter/postgres.cr#L98

Let me know if you know how to handle this case without monkey-patching PG::Decoders class.

asterite commented 5 years ago

Why do you need this? In any case, if you manager to register a decoder with a given oid some code will later fail because it relies on the oids method of the decoder.

If you really need this (but please explain) then we could make decoders receive the oid in their constructors.

mamantoha commented 5 years ago

Probably we need @imdrasil to join this conversation.

I'm not familiar with library jennifer.cr, and why it registers decoders with a given oid.

imdrasil commented 5 years ago

This was required to be able to read ENUM values from a result set. ATM all enums are returned as bytes. If you wouldn't like provide a native mechanism to get meaningful enum values from DB - Jennifer (referenced ORM) can handle this by it's own

will commented 5 years ago

The main problem with the non-standard oids (which all user created enums would be) is that the mapping has to be done per connection, or at least per remote database. Database A's oid 3456 might be an enum, but for database B it might be something else. And if you connected to two databases with the same crystal program, this could cause problems if the decoder mapping is a global as it is now.

It would be good if crystal-pg had the ability to (maybe optionally?) query the database after connection for the types and created connection-specific decoder mappings for easy ones like enums and citext and so on.

There was an early attempt https://github.com/will/crystal-pg/pull/89 but the effort would probably have to be restarted given some of the bigger changes since then.