zeroshade commented on code in PR #1152:
URL: https://github.com/apache/arrow-go/pull/1152#discussion_r3769330402
##########
arrow/flight/flightsql/driver/driver.go:
##########
@@ -620,10 +622,14 @@ func (c *Connection) Close() error {
// Begin starts and returns a new transaction.
func (c *Connection) Begin() (driver.Tx, error) {
- return c.BeginTx(context.Background(), sql.TxOptions{})
+ return c.BeginTx(context.Background(), driver.TxOptions{})
}
-func (c *Connection) BeginTx(ctx context.Context, opts sql.TxOptions)
(driver.Tx, error) {
+func (c *Connection) BeginTx(ctx context.Context, opts driver.TxOptions)
(driver.Tx, error) {
Review Comment:
Changing this exported method from `sql.TxOptions` to `driver.TxOptions` is
source-incompatible within v18. A downstream call that compiled previously now
fails:
```text
cannot use sql.TxOptions{} as driver.TxOptions in argument to
(*Connection).BeginTx
```
Please avoid silently breaking the concrete API. One option is a private
adapter returned by `Connector.Connect` that implements `driver.ConnBeginTx`
and translates its options into the existing `Connection.BeginTx` method. That
approach changes the returned concrete type, so whichever compatibility
tradeoff is chosen should be explicit.
--
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]
