lidavidm commented on code in PR #2729:
URL: https://github.com/apache/arrow-adbc/pull/2729#discussion_r2085733399
##########
go/adbc/driver/snowflake/statement.go:
##########
@@ -499,41 +516,58 @@ func (st *statement) ExecuteQuery(ctx context.Context)
(array.RecordReader, int6
st.streamBind = nil
rdr := concatReader{}
- err := rdr.Init(&bind)
+ err = rdr.Init(&bind)
if err != nil {
- return nil, -1, err
+ return
}
- return &rdr, -1, nil
+ reader = &rdr
+ return
}
- loader, err := st.cnxn.cn.QueryArrowStream(ctx, st.query)
+ var loader gosnowflake.ArrowStreamLoader
+ loader, err = st.cnxn.cn.QueryArrowStream(ctx, st.query)
if err != nil {
- return nil, -1, errToAdbcErr(adbc.StatusInternal, err)
+ err = errToAdbcErr(adbc.StatusInternal, err)
+ return
}
- rdr, err := newRecordReader(ctx, st.alloc, loader, st.queueSize,
st.prefetchConcurrency, st.useHighPrecision)
- nrec := loader.TotalRows()
- return rdr, nrec, err
+ reader, err = newRecordReader(ctx, st.alloc, loader, st.queueSize,
st.prefetchConcurrency, st.useHighPrecision)
+ nRows = loader.TotalRows()
+ return
}
// ExecuteUpdate executes a statement that does not generate a result
// set. It returns the number of rows affected if known, otherwise -1.
-func (st *statement) ExecuteUpdate(ctx context.Context) (int64, error) {
+func (st *statement) ExecuteUpdate(ctx context.Context) (numRows int64, err
error) {
+ numRows = -1
+
+ var span trace.Span
+ ctx, span = st.StartSpan(ctx, "ExecuteUpdate")
ctx = st.setQueryContext(ctx)
+ defer func() {
+ if !st.SetErrorOnSpan(span, err) {
+
span.SetAttributes(attribute.Int64("db.response.returned_rows", numRows))
Review Comment:
Also do we need a utility for this given this pattern seems to recur?
##########
go/adbc/driver/snowflake/statement.go:
##########
@@ -499,41 +516,58 @@ func (st *statement) ExecuteQuery(ctx context.Context)
(array.RecordReader, int6
st.streamBind = nil
rdr := concatReader{}
- err := rdr.Init(&bind)
+ err = rdr.Init(&bind)
if err != nil {
- return nil, -1, err
+ return
}
- return &rdr, -1, nil
+ reader = &rdr
+ return
}
- loader, err := st.cnxn.cn.QueryArrowStream(ctx, st.query)
+ var loader gosnowflake.ArrowStreamLoader
+ loader, err = st.cnxn.cn.QueryArrowStream(ctx, st.query)
if err != nil {
- return nil, -1, errToAdbcErr(adbc.StatusInternal, err)
+ err = errToAdbcErr(adbc.StatusInternal, err)
+ return
}
- rdr, err := newRecordReader(ctx, st.alloc, loader, st.queueSize,
st.prefetchConcurrency, st.useHighPrecision)
- nrec := loader.TotalRows()
- return rdr, nrec, err
+ reader, err = newRecordReader(ctx, st.alloc, loader, st.queueSize,
st.prefetchConcurrency, st.useHighPrecision)
+ nRows = loader.TotalRows()
+ return
}
// ExecuteUpdate executes a statement that does not generate a result
// set. It returns the number of rows affected if known, otherwise -1.
-func (st *statement) ExecuteUpdate(ctx context.Context) (int64, error) {
+func (st *statement) ExecuteUpdate(ctx context.Context) (numRows int64, err
error) {
+ numRows = -1
+
+ var span trace.Span
+ ctx, span = st.StartSpan(ctx, "ExecuteUpdate")
ctx = st.setQueryContext(ctx)
+ defer func() {
+ if !st.SetErrorOnSpan(span, err) {
+
span.SetAttributes(attribute.Int64("db.response.returned_rows", numRows))
Review Comment:
To be clear: does OTel accept -1 for this attribute? Since ADBC specifies -1
as the return value for when the number of rows is unknown.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
