Skip to content

Conversation

@gmartsenkov
Copy link

The decode options were not passed through to the pgo_handler:extended_query call, which cause queries done inside a transaction or the transaction call itself not to respect the rows_as_maps setting.

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 DecodeOpts from the Conn down to the extended_query call.

fixes #69

@gmartsenkov
Copy link
Author

FYI: I force pushed to add a test

Copy link
Owner

@lpil lpil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split this into a new test please, and add a comment explaining what is being tested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rows_as_map context lost in transaction connection

2 participants