feat(go/adbc): implement ADBC 1.1.0 features by lidavidm · Pull Request #700 · apache/arrow-adbc

lidavidm · 2023-05-23T19:44:29Z

ADBC_INFO_DRIVER_ADBC_VERSION
StatementExecuteSchema ([Format] Add AdbcStatementExecuteSchema #318)
ADBC_CONNECTION_OPTION_CURRENT_{CATALOG, DB_SCHEMA} ([Format] Add constants for getting/setting the 'active' catalog #319)
Get/SetOption
error_details (format: expose gRPC Status Details #755)
GetStatistics (format: add statistics for tables, columns, queries, etc. #685)
New ingest modes ([Format] Add more ingest modes #541)

zeroshade · 2023-06-28T16:43:38Z

 	return nil
 }

+func (d *database) GetOption(key string) (string, error) {


Do we want to default implement this with all the options that are accepted in SetOptions?

Yeah, I'm actually working on that now, for this and the Snowflake driver. I was originally going to do one PR per feature but at this rate it's just going to be a mega PR.

lidavidm · 2023-06-29T16:45:55Z

Updated:

Tweaks to adbc.go
Get/SetOption
Implement error details for the Flight SQL driver, allowing Flight SQL servers to return custom, rich error metadata for clients that know how to parse it

lidavidm · 2023-06-29T16:58:21Z

I think I might remove the Cancellable interface from Go, since we already have context.Context. Instead, I'll have the FFI bridge track the last context it's using, and it can implement StatementCancel() or ConnectionCancel() based on that.

lidavidm · 2023-06-29T17:34:16Z

Update:

Remove cancellable
Add a no-op test of GetStatistics (not really possible to implement for Flight SQL)

lidavidm · 2023-06-29T18:30:25Z

OK, I think things are generally complete here (except for the XDBC info). If this is good, I'll port it to the driver template as well.

lidavidm · 2023-07-10T12:48:21Z

Updated to fix the GetStatistics schema.

zeroshade · 2023-07-10T15:10:06Z

+	// the error message. The encoding of the data is driver-defined. It is
+	// suggested to use proto.Message for Protocol Buffers and error for wrapped
+	// errors.
+	Details []interface{}


why the switch to use interface{} instead of []byte? it's a bit more difficult to convert this to something usable for a C error struct for an interface{}

~Mostly so that we could keep strong typing for things like Protobufs, otherwise we would be forced to serialize them here. I'm sort of ambivalent here.
While working on Java, I also made this an array of key-value pairs; I plan to port that change here too.

So would [](pair of key, []byte) be preferable?

you can use the proto.Message interface potentially and then just serialize them later when necessary?

I don't want to elevate gRPC too much, though

lidavidm · 2023-07-10T15:26:03Z

Is the shape of the API reasonable? I'd like to tackle the FFI bridge soon

zeroshade · 2023-07-10T15:31:46Z

 Msg: err.Error(),
 Code: adbcCode,
+ // slice of proto.Message or error
+ Details: grpcStatus.Details(),


it looks like the details themselves are effectively proto.Message interfaces that we could leverage?

Yes, that's why I made it interface{} so we can easily return them directly without forcing apps to serialize/deserialize them

ideally i'd prefer to be explicit on the interface type and use proto.Message rather than interface{}

Trying to wrap this up, now it's behind an interface with wrappers for Protobuf messages.

- ADBC_INFO_DRIVER_ADBC_VERSION - StatementExecuteSchema (apache#318) - ADBC_CONNECTION_OPTION_CURRENT_{CATALOG, DB_SCHEMA} (apache#319) - Get/SetOption - error_details (apache#755) - GetStatistics (apache#685) - New ingest modes (apache#541)

lidavidm · 2023-07-10T17:31:35Z

If this looks reasonable, I'd like to merge then start on updating driver.go.tmpl

- ADBC_INFO_DRIVER_ADBC_VERSION - StatementExecuteSchema (#318) - ADBC_CONNECTION_OPTION_CURRENT_{CATALOG, DB_SCHEMA} (#319) - Get/SetOption - error_details (#755) - GetStatistics (#685) - New ingest modes (#541)

lidavidm mentioned this pull request May 24, 2023
c/driver/postgresql: infer schema by using a prepared statement #701
Closed

lidavidm force-pushed the spec-1.1.0 branch from f18f1cc to 362638dCompare June 23, 2023 18:31

lidavidm force-pushed the spec-1.1.0-execute-schema branch from 61e2aac to 5aa6a92Compare June 23, 2023 18:39

lidavidm marked this pull request as ready for review June 23, 2023 18:39

lidavidm requested a review from zeroshade as a code owner June 23, 2023 18:39

lidavidm added this to the ADBC API Specification 1.1.0 milestone Jun 26, 2023

lidavidm force-pushed the spec-1.1.0 branch from 9c9975a to b9262d5Compare June 27, 2023 17:30

lidavidm force-pushed the spec-1.1.0-execute-schema branch 2 times, most recently from 23066de to e5371a9Compare June 28, 2023 16:36

zeroshade reviewed Jun 28, 2023
View reviewed changes

lidavidm force-pushed the spec-1.1.0-execute-schema branch from e5371a9 to f6aa17bCompare June 28, 2023 21:59

lidavidm changed the title ~~feat(go/adbc/driver/flightsql): add StatementExecuteSchema~~feat(go/adbc): implement 1.1.0 featuresJun 29, 2023

lidavidm changed the title ~~feat(go/adbc): implement 1.1.0 features~~feat(go/adbc): implement ADBC 1.1.0 featuresJun 29, 2023

lidavidm force-pushed the spec-1.1.0-execute-schema branch 2 times, most recently from 3963459 to 6c9cc86Compare June 29, 2023 16:45

lidavidm force-pushed the spec-1.1.0-execute-schema branch 2 times, most recently from 742af71 to 7f7f11bCompare June 29, 2023 17:31

lidavidm force-pushed the spec-1.1.0-execute-schema branch from 7f7f11b to 237f2f0Compare June 29, 2023 18:29

lidavidm force-pushed the spec-1.1.0-execute-schema branch from 237f2f0 to e1c6991Compare July 10, 2023 12:48

lidavidm requested a review from zeroshade July 10, 2023 12:48

zeroshade reviewed Jul 10, 2023
View reviewed changes

Comment threadgo/adbc/driver/flightsql/flightsql_adbc.go Outdated

zeroshade reviewed Jul 10, 2023
View reviewed changes

Comment threadgo/adbc/driver/flightsql/flightsql_adbc.go Outdated

zeroshade reviewed Jul 10, 2023
View reviewed changes

Comment threadgo/adbc/driver/flightsql/flightsql_adbc.go Outdated

zeroshade reviewed Jul 10, 2023
View reviewed changes

lidavidm force-pushed the spec-1.1.0-execute-schema branch 2 times, most recently from 8a14461 to 1c6cab9Compare July 10, 2023 16:48

feat(go/adbc): implement 1.1.0 features
3466e2e
- ADBC_INFO_DRIVER_ADBC_VERSION - StatementExecuteSchema (apache#318) - ADBC_CONNECTION_OPTION_CURRENT_{CATALOG, DB_SCHEMA} (apache#319) - Get/SetOption - error_details (apache#755) - GetStatistics (apache#685) - New ingest modes (apache#541)

lidavidm force-pushed the spec-1.1.0-execute-schema branch from 1c6cab9 to 3466e2eCompare July 10, 2023 17:31

lidavidm merged commit e9795d3 into apache:spec-1.1.0Jul 11, 2023

lidavidm deleted the spec-1.1.0-execute-schema branch July 11, 2023 16:50

lidavidm modified the milestones: ADBC API Specification 1.1.0, ADBC Libraries 0.7.0 Aug 28, 2023

Uh oh!

Conversation

lidavidm commented May 23, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidavidm commented Jun 29, 2023

Uh oh!

lidavidm commented Jun 29, 2023

Uh oh!

lidavidm commented Jun 29, 2023

Uh oh!

lidavidm commented Jun 29, 2023

Uh oh!

lidavidm commented Jul 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lidavidm commented Jul 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidavidm commented Jul 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lidavidm commented May 23, 2023•
edited
Loading