refactor(go/adbc/driver): add common connection implementation #1572
Conversation
|
CC @zeroshade this only ports the Flight SQL side but if it seems reasonable enough I'll port the Snowflake side too. (For Snowflake, I plan to have it use its own GetObjects impl just because it's too different, but I figure in general it's easier to have drivers implement the Flight SQL GetCatalogs/GetDbSchemas/GetTables trinity and let the common driver code assemble the GetObjects structure) |
| // Will be called at most once | ||
| Close() error | ||
| // Will not be called unless autocommit is disabled | ||
| Commit(context.Context) error | ||
| CurrentCatalog() (string, bool) | ||
| CurrentDbSchema() (string, bool) | ||
| // Get boxed values for info codes | ||
| GetInfo(ctx context.Context, infoCodes []adbc.InfoCode) (map[adbc.InfoCode]interface{}, error) | ||
| // Get all info codes the driver supports (minus the 6 standard codes | ||
| // which are assumed to always be supported) | ||
| GetInfoCodes() []adbc.InfoCode | ||
| // Get all catalogs | ||
| GetObjectsCatalogs(ctx context.Context, catalog *string) ([]string, error) | ||
| GetObjectsDbSchemas(ctx context.Context, depth adbc.ObjectDepth, catalog *string, schema *string, metadataRecords []internal.Metadata) (map[string][]string, error) | ||
| GetObjectsTables(ctx context.Context, depth adbc.ObjectDepth, catalog *string, schema *string, tableName *string, columnName *string, tableType []string, metadataRecords []internal.Metadata) (map[internal.CatalogAndSchema][]internal.TableInfo, error) | ||
| GetTableSchema(ctx context.Context, catalog, dbSchema *string, tableName string) (*arrow.Schema, error) | ||
| GetTableTypes(ctx context.Context) (array.RecordReader, error) | ||
| NewStatement() (adbc.Statement, error) | ||
| ReadPartition(ctx context.Context, serializedPartition []byte) (array.RecordReader, error) | ||
| // Will not be called unless autocommit is disabled | ||
| Rollback(context.Context) error | ||
| SetAutocommit(enabled bool) error |
There was a problem hiding this comment.
can't this just be a composition using adbc.Connection ?
type ConnectionImpl interface {
adbc.GetSetOptions
Base() *ConnectionImplBase
adbc.Connection
}Or am I missing something?
There was a problem hiding this comment.
Some of the methods are different, e.g. GetInfo and the GetObjects functions
| func (base *ConnectionImplBase) Close() error { | ||
| return nil | ||
| } | ||
|
|
||
| func (base *ConnectionImplBase) Commit() error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Rather than implementing these, why not leave them unimplemented forcing anything that uses ConnectionImplBase to still need to implement them, so we don't accidentally forget them?
There was a problem hiding this comment.
See discussion in other PR - maybe we can panic (for Close) or return an error (for Commit) instead so that it satisfies the interface by default but obviously blows up?
Fixes #1105.