Copilot commented on code in PR #155:
URL: https://github.com/apache/iotdb-client-go/pull/155#discussion_r2922961652
##########
client/session.go:
##########
@@ -569,10 +569,12 @@ func (s *Session) ExecuteQueryStatement(sql string,
timeoutMs *int64) (*SessionD
request.SessionId = s.sessionId
request.StatementId = s.requestStatementId
resp, err =
s.client.ExecuteQueryStatementV2(context.Background(), &request)
- if statusErr := VerifySuccess(resp.Status); statusErr
== nil {
- return NewSessionDataSet(sql, resp.Columns,
resp.DataTypeList, resp.ColumnNameIndexMap, *resp.QueryId,
s.requestStatementId, s.client, s.sessionId, resp.QueryResult_,
resp.IgnoreTimeStamp != nil && *resp.IgnoreTimeStamp, timeoutMs,
*resp.MoreData, s.config.FetchSize, s.config.TimeZone, s.timeFactor,
resp.GetColumnIndex2TsBlockColumnIndexList())
- } else {
- return nil, statusErr
+ if err == nil && resp != nil {
+ if statusErr := VerifySuccess(resp.Status);
statusErr == nil {
+ return NewSessionDataSet(sql,
resp.Columns, resp.DataTypeList, resp.ColumnNameIndexMap, *resp.QueryId,
s.requestStatementId, s.client, s.sessionId, resp.QueryResult_,
resp.IgnoreTimeStamp != nil && *resp.IgnoreTimeStamp, timeoutMs,
*resp.MoreData, s.config.FetchSize, s.config.TimeZone, s.timeFactor,
resp.GetColumnIndex2TsBlockColumnIndexList())
+ } else {
+ return nil, statusErr
+ }
}
}
return nil, err
Review Comment:
In the reconnect branch, if the second ExecuteQueryStatementV2 call returns
(err == nil, resp == nil), this function falls through to `return nil, err` and
ends up returning (nil, nil). Please handle the `resp == nil` case explicitly
(return a non-nil error) so callers never receive a nil *SessionDataSet with a
nil error.
##########
client/sessiondataset.go:
##########
@@ -125,3 +125,7 @@ func (s *SessionDataSet) GetColumnNames() []string {
func (s *SessionDataSet) GetColumnTypes() []string {
return s.ioTDBRpcDataSet.columnTypeList
}
+
Review Comment:
`GetCurrentRowTime()` forwards the raw `IoTDBRpcDataSet.time` value. That
value is in the server/session timestamp precision (ms/us/ns depending on
`timeFactor`), not necessarily milliseconds. To avoid API confusion, consider
documenting the unit explicitly or converting to a fixed unit (e.g., always
milliseconds) and/or reflecting “raw/precision-dependent” in the method name.
```suggestion
// GetCurrentRowTime returns the raw timestamp of the current row.
// The value is in the server/session timestamp precision (milliseconds,
// microseconds, or nanoseconds) depending on the configured timeFactor.
// Callers that require a fixed unit should convert this value explicitly.
```
--
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]
