lidavidm commented on code in PR #4661:
URL: https://github.com/apache/arrow-adbc/pull/4661#discussion_r3858627264
##########
go/adbc/driver/flightsql/flightsql_statement.go:
##########
@@ -483,35 +484,43 @@ func (s *statement) SetOptionDouble(key string, value
float64) error {
// The query can then be executed with any of the Execute methods.
// For queries expected to be executed repeatedly, Prepare should be
// called before execution.
-func (s *statement) SetSqlQuery(query string) error {
+func (s *statement) SetSqlQuery(query string) (err error) {
Review Comment:
Huh, is this worth tracing? Seems like as long as the underlying close is
traced this should be fine
##########
go/adbc/driver/flightsql/record_reader.go:
##########
@@ -54,6 +53,13 @@ type reader struct {
var errReaderReleased = errors.New("record reader released")
+type recordReaderCallerContextKey struct{}
+
+func isRecordReaderSiblingCancellation(ctx context.Context) bool {
Review Comment:
I think we should document what this is checking/trying to do
##########
go/adbc/driver/flightsql/flightsql_adbc_test.go:
##########
@@ -369,9 +369,42 @@ func TestFlightSQLTracingProducesTraceFiles(t *testing.T) {
output := traceOutput.String()
require.Contains(t, output, "FlightSQL.Database.Open")
+ require.Contains(t, output, "FlightSQL.Database.Close")
require.Contains(t, output, "FlightSQL.Statement.ExecuteQuery")
}
+func TestFlightSQLTracingCleansUpAfterConstructionFailure(t *testing.T) {
Review Comment:
What exactly is this testing/how would this test fail before?
--
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]
