Respect the DecodeOpts inside a transaction#74
Conversation
|
FYI: I force pushed to add a test |
lpil
left a comment
lpil
left a comment
There was a problem hiding this comment.
Thank you! Left some notes inline, and if you could update the changelog also that would be fab!
| Res = case Pool of | ||
| {single_connection, Conn} -> | ||
| pgo_handler:extended_query(Conn, Sql, Arguments, #{}); | ||
| DecodeOpts = element(11, Conn), |
There was a problem hiding this comment.
I think this is a conn as defined in pgo_internal. Perhaps we could use the record field access syntax by importing that header file (which would be the less fragile way to do this rather than use tuple indexing). That said, I think this module is not part of PGO's public API, so I don't know if we can use it.
@tsloughter Do you have any guidance here?
Looking at the definition of extended_query/5 it's not clear to me why it takes the decode options twice, once in the conn and once as the 4th argument. Is there a way to either get the function to use the decode options from the conn, or for our code to get the decode options out of the conn?
| assert returned.rows == ["neo"] | ||
|
|
||
| Ok(returned) | ||
| }) |
There was a problem hiding this comment.
Split this into a new test please, and add a comment explaining what is being tested
|
Hoi @gmartsenkov ! I wanted to started up this message by thanking you for already making a temporary in fix for this issue with your PR. However I wanted to know the state of this PR since it's has been already few months you have starting this PR process and since your initial commits, you did not work back to solve the different review request that have been made. So I would like to know if you intent to continue working on this PR or if someone else (maybe me) could/need continue your works. For now like I said I will just appreciate your work and use your patch for my project but I would really love if this PR could be finished and merge so I can really on the official of In any case, all my best regards. |
|
Would you like to finish off this PR @gungun974 ? |
Yes, why not? ^^ I'm not fluent in Erlang, but I always enjoy opportunities to step outside my comfort zone ;) However, I'm not familiar with how to take over someone's pull request on GitHub. Do I need to fork their fork and then create a new pull request here? (Is that even possible?) |
3 participants
The decode options were not passed through to the
pgo_handler:extended_querycall, which cause queries done inside a transaction or the transaction call itself not to respect therows_as_mapssetting.I have 0 experience in Erlang and don't know how to do this. Please feel free to correct me if I'm doing something silly.
Basically trying to pass down the
DecodeOptsfrom theConndown to theextended_querycall.fixes #69