Added generic ToField and FromField classes#196
Conversation
|
This looks interesting, but for starters, why are you making a (Maybe Oid) everywhere? And I hate to say it, but we probably want to use a sentinel Oid for |
|
Once I changed type of Also I see travis is complaining (mostly because I forgot to remove my db credentials from the connection string), so there is still room for improvements. |
|
That didn't exactly answer my question, why is it even necessary (in your mind) to move to If it's in order to support the
|
|
Sorry for being unclear. The only reason for that change was bypassing typename/typeoid checks in parsers. And changing the type was the shortest way to achieve it. And I have to say nothing but that you are completely right, for my implementation looked like rather a hacky solution.
I've switched implementation to using this typeoid, but adjusted only few parsers in order to pass the tests. [1] https://www.postgresql.org/message-id/183.1302200970%40sss.pgh.pa.us |
This patch adds default generic-based implementations for
ToFieldand
FromFieldclasses. These are useful when you need to retrieveresults from sql-functions returning
setof some_typeor fromsql-queries returning columns with records.
I should mention one major change in a core module as I'm not sure
whether it's acceptable for you (and the community) or not.
The short story: in order to compose existing parsers into a record
parser we need to bypass typecheck of a
Fieldvariable. ThereforetypeOidin aFieldrecord was changed fromPQ.OidtoMaybe PQ.Oid.The long story: when we parse a record value we have no information
about types of it's fields. More precisely there are two cases:
typenameequals to real type nameWe are still able to retrieve metadata of the type, but this will be
a performance hit as we will have extra query for each execution of
the parser.
typenameequals to"record"We don't have any data, so there is nothing to do! Even worse, we
can gen
"record"instead of our custom type any time when weforget to cast results to it.
Hence to parse a record value we have to rely on a corresponding haskell
type. We already have a lot of good parsers for base types and it
would be unforgivable not to employ them :)
But there is a problem: all of them are checking
Fieldvariable foroid (or typename), which in our case is far from required value.
So (from my point of view) it's naturally to make
typeOidfieldoptional.
Aside of type unsafety there might be another pitfalls like degraded
performance or excessive memory consumption, but I'm neither expert in
haskell nor familiar with internals of
postgresql-simpleto say forsure they aren't the cases here.
All tests were passed against PostgreSQL v.9.5.1.