Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++][FlightRPC] Migrate SQL Client API to Result<> #31521

Closed
Tracked by #31074
asfimport opened this issue Apr 4, 2022 · 1 comment · Fixed by #36559
Closed
Tracked by #31074

[C++][FlightRPC] Migrate SQL Client API to Result<> #31521

asfimport opened this issue Apr 4, 2022 · 1 comment · Fixed by #36559
Assignees
@danepitkin
Labels
Component: C++ Component: FlightRPC Type: task
Milestone
13.0.0

Comments

@asfimport
Copy link

convert this API too as suggested here: #12719 (comment)

Reporter: Tobias Zagorni / @zagto

Note: This issue was originally created as ARROW-16111. Please see the migration documentation for further details.
@asfimport
Copy link
Author

Todd Farmer / @toddfarmer:
This issue was last updated over 90 days ago, which may be an indication it is no longer being actively worked. To better reflect the current state, the issue is being unassigned. Please feel free to re-take assignment of the issue if it is being actively worked, or if you plan to start that work soon.

@asfimport asfimport mentioned this issue Jan 11, 2023
Open
5 tasks
danepitkin added a commit to danepitkin/arrow that referenced this issue Jul 7, 2023
@danepitkin
apacheGH-31521: [C++][Flight] Migrate Flight SQL client to Result
eb57e88
@github-actions github-actions bot mentioned this issue Jul 7, 2023
Merged
danepitkin added a commit to danepitkin/arrow that referenced this issue Jul 7, 2023
@danepitkin
apacheGH-31521: [C++][Flight] Migrate Flight SQL client to Result
5d62d76
lidavidm pushed a commit that referenced this issue Jul 8, 2023
@danepitkin
GH-31521: [C++][Flight] Migrate Flight SQL client to Result (#36559)
33ddff3 
### Rationale for this change

Arrow introduced a `Result` return object that can be used in place of passing output parameters. Most of Flight has already been migrated. 

### Are these changes tested?

C++ tests pass, which involve the sqlite and acero example servers. Any other testing we would like to see?

```
100% tests passed, 0 tests failed out of 84

Label Time Summary:
arrow-tests         = 168.18 sec*proc (35 tests)
arrow_acero         = 106.32 sec*proc (12 tests)
arrow_compute       =  76.46 sec*proc (13 tests)
arrow_dataset       =  78.09 sec*proc (12 tests)
arrow_flight        = 123.81 sec*proc (2 tests)
arrow_flight_sql    =  19.29 sec*proc (1 test)
filesystem          =  10.39 sec*proc (2 tests)
parquet-tests       =  55.24 sec*proc (9 tests)
unittest            = 627.40 sec*proc (84 tests)

Total Test time (real) = 120.83 sec
```

### Are there any user-facing changes?

Yes. The protected methods DoPut, DoGet, and DoAction for the SQL client have been updated. DoGet already exposed a public API and that has been updated to be virtual.
* Closes: #31521

Authored-by: Dane Pitkin <dane@voltrondata.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 13.0.0 milestone Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@danepitkin danepitkin

Labels
Component: C++ Component: FlightRPC Type: task
Projects
None yet
Milestone
13.0.0
Development

Successfully merging a pull request may close this issue.

GH-31521: [C++][Flight] Migrate Flight SQL client to Result danepitkin/arrow
3 participants
@lidavidm @danepitkin @asfimport