ennuite commented on code in PR #732:
URL: https://github.com/apache/arrow-go/pull/732#discussion_r3670728243
##########
arrow/flight/flightsql/client.go:
##########
@@ -566,6 +566,7 @@ func (c *Client) LoadPreparedStatementFromResult(result
*CreatePreparedStatement
handle: result.PreparedStatementHandle,
datasetSchema: dsSchema,
paramSchema: paramSchema,
+ isUpdate: result.IsUpdate,
Review Comment:
1. I addressed this with 2 tests. In `TestPreparedStatementLoadFromResult` I
let things as they are, and tested with the new field unset, see
https://github.com/apache/arrow-go/pull/732/changes/514d3c6d1adbba18eb05faba9bcd313124849556.
Note that this test has the same nuance about `val` that I pointed out in
https://github.com/apache/arrow-go/pull/732#discussion_r3670708880
I added a new test that checks that the field is properly loaded also when
set, see
https://github.com/apache/arrow-go/pull/732/changes/a334b86f6c7dbc7e1fc33acc014e5b70b5201e23
2. For that one I purposefully followed the example set by `handle`, but I
will admit I had my own reservations while doing it.
I will not change this in this PR unless you ask me to, but in your opinion
is it worth a follow-up PR to have a defensive copy for both `handle` and
`isUpdate`? If so I can tackle that after this one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
