CurtHagenlocher opened a new issue, #4913:
URL: https://github.com/apache/arrow-adbc/issues/4913
### What happened?
**Description**
By default, `DatabaseImplBase`, `ConnectionImplBase` and `StatementImplBase`
reject every key passed to `SetOptionInt` and `SetOptionDouble`:
```
func (base *DatabaseImplBase) SetOptionInt(ctx context.Context, key
string, val int64) error {
return base.ErrorHelper.Errorf(adbc.StatusNotImplemented, "%s
'%s'", DatabaseMessageOptionUnknown, key)
}
```
Many drivers only override `SetOption(key, value string)` and parse numbers
themselves. So an option they fully support fails if it's sent as an integer or
double rather than as a string.
This causes real failures with connection profiles. Profile TOML has typed
values, and the C driver manager sends
integer and double profile options to the typed setters
(`DatabaseSetOptionInt` and `DatabaseSetOptionDouble`). For example, with the
Snowflake driver this profile:
```
profile_version = 1
driver = "snowflake"
[Options]
adbc.snowflake.sql.client_option.max_retry_count = 3
```
fails in `AdbcDatabaseInit`. The cgo layer holds typed options until init
and then calls `db.SetOptionInt`. Snowflake's databaseImpl has no
`SetOptionInt`, so the call reaches `DatabaseImplBase.SetOptionInt` and returns
NotImplemented. The same option works when written as a string
(`max_retry_count = "3"`), because Snowflake's string setter parses it with
`strconv.Atoi`. Nothing about this is specific to Snowflake: it affects any
driverbase driver that doesn't override the typed setters. It also blocks the
C# driver manager from sending typed profile options by default, since doing so
would break profiles that work today.
**Proposal**
When a driver doesn't handle a key in `SetOptionInt` or `SetOptionDouble`,
format the value and retry it through the string `SetOption`. Integers would
use `strconv.FormatInt(v, 10)` and doubles `strconv.FormatFloat(v, 'g', -1,
64)`. This matches the spec's intent that typed setters are a convenience over
the string representation, and drivers that do implement typed setters keep
their current behavior.
Because Go embedding has no virtual dispatch,
`DatabaseImplBase.SetOptionInt` can't call the driver's own SetOption. The
fallback probably belongs in the wrapper types (database, and the equivalents
for connection and statement), which hold the driver's implementation:
```
func (db *database) SetOptionInt(ctx context.Context, key string, val
int64) error {
err := db.DatabaseImpl.SetOptionInt(ctx, key, val)
if isNotImplemented(err) {
return db.DatabaseImpl.SetOption(ctx, key,
strconv.FormatInt(val, 10))
}
return err
}
```
Doubles would work the same way, as would connections and statements. If the
string setter also rejects the key, its error is returned, so unknown options
still fail with a sensible message.
**Open questions**
- Should the fallback only happen when the driver returned NotImplemented
for that key, as sketched above? Or should drivers opt in or out with a flag?
- Should SetOptionBytes also fall back to the string setter, for values
that are valid UTF-8? (I'd lean towards no.)
### Stack Trace
_No response_
### How can we reproduce the bug?
_No response_
### Environment/Setup
_No response_
--
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]
