You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "srebhan (via GitHub)" <gi...@apache.org> on 2023/04/14 16:29:01 UTC
[GitHub] [arrow] srebhan opened a new pull request, #35137: GH-35136: [Go][FlightSQL] Support backends without `CreatePreparedStatement` implemented
srebhan opened a new pull request, #35137:
URL: https://github.com/apache/arrow/pull/35137
Without this PR all backends that do not implement `CreatePreparedStatement` will error out as the `database/sql` framework tries to prepare/execute all queries if the `QueryerContext` is missing. See linked issue.
--
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] srebhan commented on pull request #35137: GH-35136: [Go][FlightSQL] Support backends without `CreatePreparedStatement` implemented
Posted by "srebhan (via GitHub)" <gi...@apache.org>.
srebhan commented on PR #35137:
URL: https://github.com/apache/arrow/pull/35137#issuecomment-1509098713
@zeroshade can you reproduce the CI error for AMD64 Debian 11 Go 1.18? Works with upstream `go version go1.18.10 linux/amd64` on my machine. :-(
--
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] github-actions[bot] commented on pull request #35137: GH-35136: [Go][FlightSQL] Support backends without `CreatePreparedStatement` implemented
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35137:
URL: https://github.com/apache/arrow/pull/35137#issuecomment-1508925137
:warning: GitHub issue #35136 **has been automatically assigned in GitHub** to PR creator.
--
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] srebhan commented on a diff in pull request #35137: GH-35136: [Go][FlightSQL] Support backends without `CreatePreparedStatement` implemented
Posted by "srebhan (via GitHub)" <gi...@apache.org>.
srebhan commented on code in PR #35137:
URL: https://github.com/apache/arrow/pull/35137#discussion_r1167189459
##########
go/arrow/flight/flightsql/driver/driver.go:
##########
@@ -442,11 +442,54 @@ func (c *Connection) PrepareContext(ctx context.Context, query string) (driver.S
return nil, err
}
- return &Stmt{
+ s := &Stmt{
stmt: stmt,
client: c.client,
timeout: c.timeout,
- }, nil
+ }
+
+ return s, nil
+}
+
+func (c *Connection) QueryContext(ctx context.Context, query string, args []driver.NamedValue) (driver.Rows, error) {
+ if len(args) > 0 {
+ // We cannot pass arguments to the client so we skip a direct query.
+ // This will force the sql-framework to prepare and execute queries.
+ return nil, driver.ErrSkip
+ }
+
+ if _, set := ctx.Deadline(); !set && c.timeout > 0 {
+ var cancel context.CancelFunc
+ ctx, cancel = context.WithTimeout(ctx, c.timeout)
+ defer cancel()
+ }
+
+ info, err := c.client.Execute(ctx, query)
+ if err != nil {
+ return nil, err
+ }
+
+ rows := Rows{}
+ for _, endpoint := range info.Endpoint {
+ reader, err := c.client.DoGet(ctx, endpoint.GetTicket())
Review Comment:
Refactored the code a bit as `func (s *Stmt) QueryContext(ctx context.Context, args []driver.NamedValue) (driver.Rows, error)` had the same issue.
--
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] ursabot commented on pull request #35137: GH-35136: [Go][FlightSQL] Support backends without `CreatePreparedStatement` implemented
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35137:
URL: https://github.com/apache/arrow/pull/35137#issuecomment-1511770519
Benchmark runs are scheduled for baseline = b63463c62881d098e9e262e52ca2bd29a416e55d and contender = bb69b9f2d6177304d7b7a2964aa68a7a8b17514b. bb69b9f2d6177304d7b7a2964aa68a7a8b17514b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/1dac385aad754a90bd2303a632d6e480...15b253e28fde4d539ba5940da4588b8b/)
[Failed] [test-mac-arm](https://conbench.ursa.dev/compare/runs/b0a87ca85a3e4dafae4d04a5e3bf133d...d9e97fd3a2f7406ab52832596fa653b8/)
[Finished :arrow_down:0.26% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/0784af810f54448bb6353fa119c031bc...ea036e76612d4e66b929e1a5024072ae/)
[Finished :arrow_down:0.34% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/9ab58e69a88f44d2ad57b28ffcb13ead...2541e624959f48d1969d67926b9d8bac/)
Buildkite builds:
[Finished] [`bb69b9f2` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2721)
[Failed] [`bb69b9f2` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2755)
[Finished] [`bb69b9f2` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2719)
[Finished] [`bb69b9f2` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2746)
[Finished] [`b63463c6` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2720)
[Failed] [`b63463c6` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2754)
[Finished] [`b63463c6` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2718)
[Finished] [`b63463c6` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2745)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] zeroshade commented on a diff in pull request #35137: GH-35136: [Go][FlightSQL] Support backends without `CreatePreparedStatement` implemented
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #35137:
URL: https://github.com/apache/arrow/pull/35137#discussion_r1167147996
##########
go/arrow/flight/flightsql/driver/driver.go:
##########
@@ -442,11 +442,54 @@ func (c *Connection) PrepareContext(ctx context.Context, query string) (driver.S
return nil, err
}
- return &Stmt{
+ s := &Stmt{
stmt: stmt,
client: c.client,
timeout: c.timeout,
- }, nil
+ }
+
+ return s, nil
+}
+
+func (c *Connection) QueryContext(ctx context.Context, query string, args []driver.NamedValue) (driver.Rows, error) {
+ if len(args) > 0 {
+ // We cannot pass arguments to the client so we skip a direct query.
+ // This will force the sql-framework to prepare and execute queries.
+ return nil, driver.ErrSkip
+ }
+
+ if _, set := ctx.Deadline(); !set && c.timeout > 0 {
+ var cancel context.CancelFunc
+ ctx, cancel = context.WithTimeout(ctx, c.timeout)
+ defer cancel()
+ }
+
+ info, err := c.client.Execute(ctx, query)
+ if err != nil {
+ return nil, err
+ }
+
+ rows := Rows{}
+ for _, endpoint := range info.Endpoint {
+ reader, err := c.client.DoGet(ctx, endpoint.GetTicket())
Review Comment:
you need `defer reader.Release()`
--
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] zeroshade merged pull request #35137: GH-35136: [Go][FlightSQL] Support backends without `CreatePreparedStatement` implemented
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #35137:
URL: https://github.com/apache/arrow/pull/35137
--
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] github-actions[bot] commented on pull request #35137: GH-35136: [Go][FlightSQL] Support backends without `CreatePreparedStatement` implemented
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35137:
URL: https://github.com/apache/arrow/pull/35137#issuecomment-1508925102
* Closes: #35136
--
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