feat(go/adbc): implement ADBC 1.1.0 features #700

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

go/adbc/driver/flightsql/flightsql_adbc.go

 			Code: adbc.StatusInvalidArgument,
 		}
 	}

 	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

go/adbc/adbc.go

+	// 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

go/adbc/driver/flightsql/flightsql_adbc.go

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

go/adbc/driver/flightsql/utils.go

@@ -71,5 +73,7 @@ func adbcFromFlightStatus(err error) error {
 	return adbc.Error{
 		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 362638d Compare June 23, 2023 18:31

lidavidm force-pushed the spec-1.1.0-execute-schema branch from 61e2aac to 5aa6a92 Compare 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 b9262d5 Compare June 27, 2023 17:30

lidavidm force-pushed the spec-1.1.0-execute-schema branch 2 times, most recently from 23066de to e5371a9 Compare 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 f6aa17b Compare June 28, 2023 21:59

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

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

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

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

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

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

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

zeroshade reviewed Jul 10, 2023

View reviewed changes

go/adbc/driver/flightsql/flightsql_adbc.go Outdated Show resolved Hide resolved

zeroshade reviewed Jul 10, 2023

View reviewed changes

go/adbc/driver/flightsql/flightsql_adbc.go Outdated Show resolved Hide resolved

zeroshade reviewed Jul 10, 2023

View reviewed changes

go/adbc/driver/flightsql/flightsql_adbc.go Outdated Show resolved Hide resolved

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 1c6cab9 Compare 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 3466e2e Compare July 10, 2023 17:31

lidavidm merged commit e9795d3 into apache:spec-1.1.0 Jul 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(go/adbc): implement ADBC 1.1.0 features #700

feat(go/adbc): implement ADBC 1.1.0 features #700

lidavidm commented May 23, 2023 •

edited

Loading

zeroshade Jun 28, 2023

lidavidm Jun 28, 2023

lidavidm commented Jun 29, 2023

lidavidm commented Jun 29, 2023

lidavidm commented Jun 29, 2023

lidavidm commented Jun 29, 2023

lidavidm commented Jul 10, 2023

zeroshade Jul 10, 2023

lidavidm Jul 10, 2023

lidavidm Jul 10, 2023

zeroshade Jul 10, 2023

lidavidm Jul 10, 2023

lidavidm commented Jul 10, 2023

zeroshade Jul 10, 2023

lidavidm Jul 10, 2023

zeroshade Jul 10, 2023

lidavidm Jul 10, 2023

lidavidm commented Jul 10, 2023

feat(go/adbc): implement ADBC 1.1.0 features #700

feat(go/adbc): implement ADBC 1.1.0 features #700

Conversation

lidavidm commented May 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lidavidm commented Jun 29, 2023

lidavidm commented Jun 29, 2023

lidavidm commented Jun 29, 2023

lidavidm commented Jun 29, 2023

lidavidm commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lidavidm commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lidavidm commented Jul 10, 2023

lidavidm commented May 23, 2023 •

edited

Loading