You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2023/05/23 19:44:31 UTC

[GitHub] [arrow-adbc] lidavidm opened a new pull request, #700: feat(go/adbc/driver/flightsql): add StatementExecuteSchema

lidavidm opened a new pull request, #700:
URL: https://github.com/apache/arrow-adbc/pull/700

   Requires #692, #698.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258516161


##########
go/adbc/adbc.go:
##########
@@ -58,13 +58,15 @@ type Error struct {
 	// SqlState is a SQLSTATE error code, if provided, as defined
 	// by the SQL:2003 standard. If not set, it will be "\0\0\0\0\0"
 	SqlState [5]byte
-	// Details is an array of additional driver-specific binary error details.
+	// Details is an array of additional driver-specific error details.
 	//
 	// This allows drivers to return custom, structured error information (for
 	// example, JSON or Protocol Buffers) that can be optionally parsed by
 	// clients, beyond the standard Error fields, without having to encode it in
-	// the error message.  The encoding of the data is driver-defined.
-	Details [][]byte
+	// 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{}

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



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258445390


##########
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(),

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



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258420886


##########
go/adbc/driver/flightsql/flightsql_adbc.go:
##########
@@ -742,39 +973,16 @@ func (c *cnxn) SetOption(key, value string) error {
 
 	switch key {
 	case OptionTimeoutFetch:

Review Comment:
   any reason to use `fallthrough` as opposed to just having the multiple cases?
   
   `case OptionTimeoutFetch, OptionTimeoutQuery, OptionTimeoutUpdate:` ?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #700: feat(go/adbc/driver/flightsql): add StatementExecuteSchema

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1245495875


##########
go/adbc/driver/flightsql/flightsql_adbc.go:
##########
@@ -369,14 +370,63 @@ func (d *database) SetOptions(cnOptions map[string]string) error {
 			continue
 		}
 		return adbc.Error{
-			Msg:  fmt.Sprintf("Unknown database option '%s'", key),
+			Msg:  fmt.Sprintf("[Flight SQL] Unknown database option '%s'", key),
 			Code: adbc.StatusInvalidArgument,
 		}
 	}
 
 	return nil
 }
 
+func (d *database) GetOption(key string) (string, error) {

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



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258442700


##########
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(),

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



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#issuecomment-1613576140

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


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258505507


##########
go/adbc/adbc.go:
##########
@@ -58,13 +58,15 @@ type Error struct {
 	// SqlState is a SQLSTATE error code, if provided, as defined
 	// by the SQL:2003 standard. If not set, it will be "\0\0\0\0\0"
 	SqlState [5]byte
-	// Details is an array of additional driver-specific binary error details.
+	// Details is an array of additional driver-specific error details.
 	//
 	// This allows drivers to return custom, structured error information (for
 	// example, JSON or Protocol Buffers) that can be optionally parsed by
 	// clients, beyond the standard Error fields, without having to encode it in
-	// the error message.  The encoding of the data is driver-defined.
-	Details [][]byte
+	// 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{}

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



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#issuecomment-1629193113

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


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#issuecomment-1628892193

   Updated to fix the GetStatistics schema.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm merged pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258590902


##########
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(),

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



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258418071


##########
go/adbc/adbc.go:
##########
@@ -58,13 +58,15 @@ type Error struct {
 	// SqlState is a SQLSTATE error code, if provided, as defined
 	// by the SQL:2003 standard. If not set, it will be "\0\0\0\0\0"
 	SqlState [5]byte
-	// Details is an array of additional driver-specific binary error details.
+	// Details is an array of additional driver-specific error details.
 	//
 	// This allows drivers to return custom, structured error information (for
 	// example, JSON or Protocol Buffers) that can be optionally parsed by
 	// clients, beyond the standard Error fields, without having to encode it in
-	// the error message.  The encoding of the data is driver-defined.
-	Details [][]byte
+	// 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{}

Review Comment:
   ~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.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258421852


##########
go/adbc/driver/flightsql/flightsql_adbc.go:
##########
@@ -742,39 +973,16 @@ func (c *cnxn) SetOption(key, value string) error {
 
 	switch key {
 	case OptionTimeoutFetch:

Review Comment:
   i can update, not a big deal



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#issuecomment-1613518557

   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


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#issuecomment-1613610904

   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.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#issuecomment-1613533796

   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.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258414852


##########
go/adbc/adbc.go:
##########
@@ -58,13 +58,15 @@ type Error struct {
 	// SqlState is a SQLSTATE error code, if provided, as defined
 	// by the SQL:2003 standard. If not set, it will be "\0\0\0\0\0"
 	SqlState [5]byte
-	// Details is an array of additional driver-specific binary error details.
+	// Details is an array of additional driver-specific error details.
 	//
 	// This allows drivers to return custom, structured error information (for
 	// example, JSON or Protocol Buffers) that can be optionally parsed by
 	// clients, beyond the standard Error fields, without having to encode it in
-	// the error message.  The encoding of the data is driver-defined.
-	Details [][]byte
+	// 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{}

Review Comment:
   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{}`



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258433298


##########
go/adbc/driver/flightsql/flightsql_adbc.go:
##########
@@ -827,6 +1035,45 @@ func (c *cnxn) SetOption(key, value string) error {
 	return nil
 }
 
+func (c *cnxn) SetOptionBytes(key string, value []byte) error {
+	return adbc.Error{
+		Msg:  "[Flight SQL] unknown connection option",
+		Code: adbc.StatusNotImplemented,
+	}
+}
+
+func (c *cnxn) SetOptionInt(key string, value int64) error {
+	switch key {
+	case OptionTimeoutFetch:
+		fallthrough
+	case OptionTimeoutQuery:
+		fallthrough
+	case OptionTimeoutUpdate:
+		return c.timeouts.setTimeout(key, float64(value))

Review Comment:
   same question as earlier, any reason to use `fallthrough` over just multiple values in the case? 
   
   `case OptionTimeoutFetch, OptionTimeoutQuery, OptionTimeoutUpdate:`



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258511276


##########
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(),

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



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] zeroshade commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258422992


##########
go/adbc/driver/flightsql/flightsql_adbc.go:
##########
@@ -742,39 +973,16 @@ func (c *cnxn) SetOption(key, value string) error {
 
 	switch key {
 	case OptionTimeoutFetch:
-		timeout, err := getTimeoutOptionValue(value)
-		if err != nil {
-			return adbc.Error{
-				Msg: fmt.Sprintf("invalid timeout option value %s = %s : %s",
-					OptionTimeoutFetch, value, err.Error()),
-				Code: adbc.StatusInvalidArgument,
-			}
-		}
-		c.timeouts.fetchTimeout = timeout
+		fallthrough
 	case OptionTimeoutQuery:
-		timeout, err := getTimeoutOptionValue(value)
-		if err != nil {
-			return adbc.Error{
-				Msg: fmt.Sprintf("invalid timeout option value %s = %s : %s",
-					OptionTimeoutFetch, value, err.Error()),
-				Code: adbc.StatusInvalidArgument,
-			}
-		}
-		c.timeouts.queryTimeout = timeout
+		fallthrough
 	case OptionTimeoutUpdate:
-		timeout, err := getTimeoutOptionValue(value)
-		if err != nil {
-			return adbc.Error{
-				Msg: fmt.Sprintf("invalid timeout option value %s = %s : %s",
-					OptionTimeoutFetch, value, err.Error()),
-				Code: adbc.StatusInvalidArgument,
-			}
-		}
-		c.timeouts.updateTimeout = timeout
+		return c.timeouts.setTimeoutString(key, value)
 	case adbc.OptionKeyAutoCommit:
 		autocommit := true
 		switch value {
 		case adbc.OptionValueEnabled:
+			autocommit = true

Review Comment:
   is this needed since we default it to true?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1258419640


##########
go/adbc/adbc.go:
##########
@@ -58,13 +58,15 @@ type Error struct {
 	// SqlState is a SQLSTATE error code, if provided, as defined
 	// by the SQL:2003 standard. If not set, it will be "\0\0\0\0\0"
 	SqlState [5]byte
-	// Details is an array of additional driver-specific binary error details.
+	// Details is an array of additional driver-specific error details.
 	//
 	// This allows drivers to return custom, structured error information (for
 	// example, JSON or Protocol Buffers) that can be optionally parsed by
 	// clients, beyond the standard Error fields, without having to encode it in
-	// the error message.  The encoding of the data is driver-defined.
-	Details [][]byte
+	// 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{}

Review Comment:
   So would `[](pair of key, []byte)` be preferable?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on pull request #700: feat(go/adbc): implement ADBC 1.1.0 features

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#issuecomment-1629409619

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


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #700: feat(go/adbc/driver/flightsql): add StatementExecuteSchema

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #700:
URL: https://github.com/apache/arrow-adbc/pull/700#discussion_r1245545018


##########
go/adbc/driver/flightsql/flightsql_adbc.go:
##########
@@ -369,14 +370,63 @@ func (d *database) SetOptions(cnOptions map[string]string) error {
 			continue
 		}
 		return adbc.Error{
-			Msg:  fmt.Sprintf("Unknown database option '%s'", key),
+			Msg:  fmt.Sprintf("[Flight SQL] Unknown database option '%s'", key),
 			Code: adbc.StatusInvalidArgument,
 		}
 	}
 
 	return nil
 }
 
+func (d *database) GetOption(key string) (string, error) {

Review Comment:
   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.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org