This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new aaafd49564 fix(go/driver/internal/driverbase): fall back to SetOption
for unhandled typed options (#4924)
aaafd49564 is described below
commit aaafd49564b3fddf15282e68f2075c730e3f4d35
Author: ump45nose <[email protected]>
AuthorDate: Fri Oct 30 11:39:24 2026 +0800
fix(go/driver/internal/driverbase): fall back to SetOption for unhandled
typed options (#4924)
Following the proposal in the issue, the fallback lives in the
`database`, `connection` and `statement` wrappers, since they hold the
driver's implementation (`DatabaseImplBase` can't dispatch to the
driver's own `SetOption`).
This PR was prepared with AI assistance.
Closes #4913.
---------
Signed-off-by: Vincent <[email protected]>
---
go/driver/internal/driverbase/connection.go | 21 +++
go/driver/internal/driverbase/database.go | 24 +++
go/driver/internal/driverbase/error.go | 8 +
go/driver/internal/driverbase/statement.go | 21 +++
.../internal/driverbase/typed_options_test.go | 163 +++++++++++++++++++++
5 files changed, 237 insertions(+)
diff --git a/go/driver/internal/driverbase/connection.go
b/go/driver/internal/driverbase/connection.go
index 977ae02b30..e6f33733f1 100644
--- a/go/driver/internal/driverbase/connection.go
+++ b/go/driver/internal/driverbase/connection.go
@@ -24,6 +24,7 @@ import (
"errors"
"fmt"
"log/slog"
+ "strconv"
"strings"
"github.com/apache/arrow-adbc/go/adbc"
@@ -562,6 +563,26 @@ func (cnxn *connection) SetOption(key string, val string)
error {
return cnxn.ConnectionImpl.SetOption(key, val)
}
+// SetOptionInt sets an integer option, falling back to SetOption with the
+// formatted value when the driver does not handle key as an integer.
+func (cnxn *connection) SetOptionInt(key string, val int64) error {
+ err := cnxn.ConnectionImpl.SetOptionInt(key, val)
+ if isNotImplemented(err) {
+ return cnxn.SetOption(key, strconv.FormatInt(val, 10))
+ }
+ return err
+}
+
+// SetOptionDouble sets a double option, falling back to SetOption with the
+// formatted value when the driver does not handle key as a double.
+func (cnxn *connection) SetOptionDouble(key string, val float64) error {
+ err := cnxn.ConnectionImpl.SetOptionDouble(key, val)
+ if isNotImplemented(err) {
+ return cnxn.SetOption(key, strconv.FormatFloat(val, 'g', -1,
64))
+ }
+ return err
+}
+
func (cnxn *connection) GetInfo(ctx context.Context, infoCodes
[]adbc.InfoCode) (array.RecordReader, error) {
if cnxn.driverInfoPreparer != nil {
if err := cnxn.driverInfoPreparer.PrepareDriverInfo(ctx,
infoCodes); err != nil {
diff --git a/go/driver/internal/driverbase/database.go
b/go/driver/internal/driverbase/database.go
index 50693a66bd..ba9847659f 100644
--- a/go/driver/internal/driverbase/database.go
+++ b/go/driver/internal/driverbase/database.go
@@ -23,6 +23,7 @@ import (
"io"
"log/slog"
"os"
+ "strconv"
"strings"
"sync"
"time"
@@ -252,6 +253,29 @@ func NewDatabase(impl DatabaseImpl) Database {
}
}
+// SetOptionInt sets an integer option. If the driver does not handle key as
+// an integer, the value is formatted and passed to SetOption instead, since
+// the typed setters are a convenience over the string representation.
+// Drivers that only override SetOption and parse numbers themselves then
+// accept integer options too (e.g. typed values from connection profiles).
+func (db *database) SetOptionInt(key string, val int64) error {
+ err := db.DatabaseImpl.SetOptionInt(key, val)
+ if isNotImplemented(err) {
+ return db.SetOption(key, strconv.FormatInt(val, 10))
+ }
+ return err
+}
+
+// SetOptionDouble sets a double option, falling back to SetOption like
+// SetOptionInt when the driver does not handle key as a double.
+func (db *database) SetOptionDouble(key string, val float64) error {
+ err := db.DatabaseImpl.SetOptionDouble(key, val)
+ if isNotImplemented(err) {
+ return db.SetOption(key, strconv.FormatFloat(val, 'g', -1, 64))
+ }
+ return err
+}
+
func (db *database) SetLogger(logger *slog.Logger) {
if logger != nil {
db.Base().Logger = logger
diff --git a/go/driver/internal/driverbase/error.go
b/go/driver/internal/driverbase/error.go
index cb60c17e40..f1f2a1a866 100644
--- a/go/driver/internal/driverbase/error.go
+++ b/go/driver/internal/driverbase/error.go
@@ -18,6 +18,7 @@
package driverbase
import (
+ "errors"
"fmt"
"github.com/apache/arrow-adbc/go/adbc"
@@ -35,3 +36,10 @@ func (helper *ErrorHelper) Errorf(code adbc.Status, message
string, format ...in
Msg: fmt.Sprintf("[%s] %s", helper.DriverName, msg),
}
}
+
+// isNotImplemented reports whether err is an ADBC error with status
+// NotImplemented, which is how the *ImplBase types reject unknown options.
+func isNotImplemented(err error) bool {
+ adbcErr, ok := errors.AsType[adbc.Error](err)
+ return ok && adbcErr.Code == adbc.StatusNotImplemented
+}
diff --git a/go/driver/internal/driverbase/statement.go
b/go/driver/internal/driverbase/statement.go
index 5fa43da916..1f964d3139 100644
--- a/go/driver/internal/driverbase/statement.go
+++ b/go/driver/internal/driverbase/statement.go
@@ -19,6 +19,7 @@ package driverbase
import (
"context"
+ "strconv"
"strings"
"github.com/apache/arrow-adbc/go/adbc"
@@ -71,6 +72,26 @@ func NewStatement(impl StatementImpl) Statement {
}
}
+// SetOptionInt sets an integer option, falling back to SetOption with the
+// formatted value when the driver does not handle key as an integer.
+func (st *statement) SetOptionInt(key string, value int64) error {
+ err := st.StatementImpl.SetOptionInt(key, value)
+ if isNotImplemented(err) {
+ return st.SetOption(key, strconv.FormatInt(value, 10))
+ }
+ return err
+}
+
+// SetOptionDouble sets a double option, falling back to SetOption with the
+// formatted value when the driver does not handle key as a double.
+func (st *statement) SetOptionDouble(key string, value float64) error {
+ err := st.StatementImpl.SetOptionDouble(key, value)
+ if isNotImplemented(err) {
+ return st.SetOption(key, strconv.FormatFloat(value, 'g', -1,
64))
+ }
+ return err
+}
+
func (st *StatementImplBase) SetOption(key, value string) error {
switch strings.ToLower(key) {
case adbc.OptionKeyTelemetryTraceParent:
diff --git a/go/driver/internal/driverbase/typed_options_test.go
b/go/driver/internal/driverbase/typed_options_test.go
new file mode 100644
index 0000000000..537c35e09f
--- /dev/null
+++ b/go/driver/internal/driverbase/typed_options_test.go
@@ -0,0 +1,163 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase_test
+
+import (
+ "context"
+ "strings"
+ "testing"
+
+ "github.com/apache/arrow-adbc/go/adbc"
+ "github.com/apache/arrow-adbc/go/adbc/validation"
+ "github.com/apache/arrow-adbc/go/driver/internal/driverbase"
+ "github.com/apache/arrow-go/v18/arrow/memory"
+ "github.com/stretchr/testify/require"
+)
+
+const (
+ // Keys with this prefix are only understood by the string setters,
+ // like a driver that parses numbers from SetOption itself.
+ stringOnlyKeyPrefix = "string."
+ // This key is handled natively by the integer setter.
+ nativeIntKey = "native.int"
+)
+
+// typedOptionsDatabase, typedOptionsConnection and typedOptionsStatement
+// record the options they accept so tests can see which setter got a value.
+type typedOptionsDatabase struct {
+ driverbase.DatabaseImplBase
+ opts map[string]string
+}
+
+func (d *typedOptionsDatabase) SetOption(key, value string) error {
+ if strings.HasPrefix(key, stringOnlyKeyPrefix) {
+ d.opts[key] = value
+ return nil
+ }
+ return d.DatabaseImplBase.SetOption(key, value)
+}
+
+func (d *typedOptionsDatabase) SetOptionInt(key string, value int64) error {
+ if key == nativeIntKey {
+ d.opts[key] = "native"
+ return nil
+ }
+ return d.DatabaseImplBase.SetOptionInt(key, value)
+}
+
+func (d *typedOptionsDatabase) Open(ctx context.Context) (adbc.Connection,
error) {
+ cnxn := &typedOptionsConnection{
+ ConnectionImplBase:
driverbase.NewConnectionImplBase(&d.DatabaseImplBase),
+ opts: d.opts,
+ }
+ return driverbase.NewConnectionBuilder(cnxn).Connection(), nil
+}
+
+type typedOptionsConnection struct {
+ driverbase.ConnectionImplBase
+ opts map[string]string
+}
+
+func (c *typedOptionsConnection) SetOption(key, value string) error {
+ if strings.HasPrefix(key, stringOnlyKeyPrefix) {
+ c.opts[key] = value
+ return nil
+ }
+ return c.ConnectionImplBase.SetOption(key, value)
+}
+
+func (c *typedOptionsConnection) NewStatement() (adbc.Statement, error) {
+ stmt := &typedOptionsStatement{
+ statement: statement{StatementImplBase:
driverbase.NewStatementImplBase(c.Base(), c.ErrorHelper)},
+ opts: c.opts,
+ }
+ return driverbase.NewStatement(stmt), nil
+}
+
+type typedOptionsStatement struct {
+ statement
+ opts map[string]string
+}
+
+func (s *typedOptionsStatement) SetOption(key, value string) error {
+ if strings.HasPrefix(key, stringOnlyKeyPrefix) {
+ s.opts[key] = value
+ return nil
+ }
+ return s.StatementImplBase.SetOption(key, value)
+}
+
+func (s *typedOptionsStatement) Close() error { return nil }
+
+func TestTypedOptionsFallBackToStringSetter(t *testing.T) {
+ ctx := context.Background()
+ alloc := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer alloc.AssertSize(t, 0)
+
+ drv :=
driverbase.NewDriverImplBase(driverbase.DefaultDriverInfo("MockDriver"), alloc)
+ dbBase, err := driverbase.NewDatabaseImplBase(ctx, &drv,
driverbase.TracingOptions{})
+ require.NoError(t, err)
+ opts := map[string]string{}
+ db := driverbase.NewDatabase(&typedOptionsDatabase{DatabaseImplBase:
dbBase, opts: opts})
+ defer validation.CheckedClose(t, db)
+
+ cnxn, err := db.Open(ctx)
+ require.NoError(t, err)
+ defer validation.CheckedClose(t, cnxn)
+
+ stmt, err := cnxn.NewStatement()
+ require.NoError(t, err)
+ defer validation.CheckedClose(t, stmt)
+
+ for _, tc := range []struct {
+ name string
+ getset adbc.GetSetOptions
+ }{
+ {"database", db},
+ {"connection", cnxn.(adbc.GetSetOptions)},
+ {"statement", stmt.(adbc.GetSetOptions)},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ intKey := stringOnlyKeyPrefix + tc.name + ".int"
+ require.NoError(t, tc.getset.SetOptionInt(intKey, -3))
+ require.Equal(t, "-3", opts[intKey])
+
+ doubleKey := stringOnlyKeyPrefix + tc.name + ".double"
+ require.NoError(t, tc.getset.SetOptionDouble(doubleKey,
0.25))
+ require.Equal(t, "0.25", opts[doubleKey])
+ require.NoError(t, tc.getset.SetOptionDouble(doubleKey,
1e21))
+ require.Equal(t, "1e+21", opts[doubleKey])
+
+ // Keys that neither setter understands are still
rejected.
+ var adbcErr adbc.Error
+ err := tc.getset.SetOptionInt(OptionKeyUnrecognized, 1)
+ require.ErrorAs(t, err, &adbcErr)
+ require.Equal(t, adbc.StatusNotImplemented,
adbcErr.Code)
+ require.Contains(t, adbcErr.Msg,
"'"+OptionKeyUnrecognized+"'")
+
+ err = tc.getset.SetOptionDouble(OptionKeyUnrecognized,
1.5)
+ require.ErrorAs(t, err, &adbcErr)
+ require.Equal(t, adbc.StatusNotImplemented,
adbcErr.Code)
+ })
+ }
+
+ // A typed setter that handles the key itself is not routed through
+ // SetOption.
+ require.NoError(t, db.SetOptionInt(nativeIntKey, 7))
+ require.Equal(t, "native", opts[nativeIntKey])
+}
