database/sql: documentation: ambiguity with contexts and Rows.Next

[kevin-matthew]

Rows.Next is the query equivalent of a stream's Read(...) in the fact that these functions make external API calls that would normally require a context be supplied, but instead implicitly use the context supplied in the method that established the stream (ie, QueryContext and Listen respectively).

This implicit behavior is not documented on QueryContext nor Next. So without diving into driver code, one wouldn't know if canceling the context passed into the QueryContext would actually effect Next. Some drivers may download 100% of the result rows before Query is returned, others may download parts at a time in the background, and others may only download rows only when Next is called.

This causes the problem when it comes to streaming results while also processing them by other means that require contexts.

type RowProcessor interface {
    // OpenProcessor will execute the query and will read row-by-row every time
    // ProcessRow is called.
    OpenProcessor(ctx context.Context, sql string, args ...any) (err error)

    // ProcessRow will read the next row from the query, execute some other
    // queries to process it, and return non-nil when the next row is ready to
    // processed.
    ProcessRow(ctx context.Context) (err error)
}

In the above example, OpenProcessor uses a context to execute the original query, and ProcessRow must take a context to execute auxiliary queries/api calls with. But what is confusing is that ProcessRow is still affected by the context passed into OpenProcessor. If the user cancels the context used for OpenProcessor, that may have ProcessRow fail in subsequent calls. Problems arise because OpenProcessor may have a tight deadline to perform its basic operations, whereas a longer deadline is appropriate for each call to ProcessRow, so that first tight deadline may expire well before all the rows have been processed. Thus, the context used for OpenProcessor must be valid for the entire processing duration. This will cause problems, as that overall context is expected to be minutes long, so if OpenProcessor hangs, the user would have to wait those same minutes.

In conclusion, I think Next() should have a NextContext(context.Context) alternative. This will allow the original context used for QueryContext to be cancelled before the results are fully processed. It provides a way to utilize a context exclusively for initializing a row stream rather than linger around for the entirety of the steam.

[gabyhelp]

Related Issues

• database/sql: update documentation to explain context cancellation in more detail #27962 (closed)
• database/sql: add documentation about Context methods #21244 (closed)
• database/sql: Go 2: Is context cancellation on QueryContext meant to cancel row.Scan? #28842 (closed)
• database/sql: Context cancellation doesn't #18168 (closed)
• database/sql: no way to protect driver.Connector.Connect(), driver.Conn.Close(), driver.Stmt.Close() from blocking #38185 (closed)
• database/sql: rows.Err() should probably return the last context.Err() #18961 (closed)
• database/sql: sql.RawBytes is not compatible with contexts #60304 (closed)
• proposal: database/sql: Rows.Scan should return lasterr if present #24431 (closed)
• database/sql: if a context is cancelled, then pass back that error #25829 (closed)

Related Code Changes

• database/sql: do not bypass the driver locks with Context methods

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[kevin-matthew]

Reading through the related issues, there is many proposals for clearer documentation on what the context does in relationship with drivers. Few of them have deduced an actual fix.

@kardianos looks like you are best fit for this discussion. This would be an API change, maybe best for v2, I'm not sure. Let me try to issue a concrete interface method

// NextContext behaves as Next but includes a context that if cancelled will cause 
// false to be returned. Err will subsequently return the context's cancel error.
NextContext(context.Context) bool

Furthermore, QueryContext would need clarification that implies that the passed in context does can be cancelled post-call without affecting NextContext (provided, this implication is implicit among golang developers as a context is seldom persistent in structures)

[rittneje]

Wouldn't this require a corresponding change in sql/driver.Rows? (Or in particular, an optional "extension" interface around it.)

[kevin-matthew]

@rittneje Yes you're exactly right. Sorry for focusing exclusively on the connection interface.