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.

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

Implement Basic FlightSQL Client #1413 #1616

Closed
wants to merge 21 commits into from
Closed

Implement Basic FlightSQL Client #1413 #1616

wants to merge 21 commits into from

Conversation

timvw
Copy link

@timvw timvw commented Apr 24, 2022

Which issue does this PR close?

Closes #1413

Client application replicates functionality in FlightSqlClientDemoAp and works against FlightSqlExample

cargo run --example=client --features="flight-sql-experimental" Execute "select * from app.inttable"

+----+--------------+-------+-----------+
| ID | KEYNAME      | VALUE | FOREIGNID |
+----+--------------+-------+-----------+
| 1  | one          | 1     | 1         |
| 2  | zero         | 0     | 1         |
| 3  | negative one | -1    | 1         |
+----+--------------+-------+-----------+
+----+---------+-------+-----------+
| ID | KEYNAME | VALUE | FOREIGNID |
+----+---------+-------+-----------+
+----+---------+-------+-----------+

@github-actions github-actions bot added the arrow-flight Changes to the arrow-flight crate label Apr 24, 2022
@timvw
Copy link
Author

timvw commented Apr 24, 2022

Some cleanup/suggestions for cleanup are very welcome (I'm still very new to writing rust code, most definitely will want to make the error handling more rust idiomatic..)

@timvw timvw mentioned this pull request Apr 24, 2022
2 tasks
@wangfenjin
Copy link
Contributor

Thanks for the PR!

Actually I think what we expected is a client API that can help user to build the example you showed in here. Such as we add a src/sql/client.rs to implement the API similar to cpp-client-api, and then use the client api in examples/

@timvw
Copy link
Author

timvw commented Apr 25, 2022

Got it. Will try to find some time to factor out the (ipc/conversion) parts and see what would make sense in a client-api..

@wangfenjin
Copy link
Contributor

@timvw I pushed my WIP code for your reference, you can take a look if you are interested.

wangfenjin@7da031d

@github-actions github-actions bot added the arrow Changes to the arrow crate label Apr 26, 2022
@timvw
Copy link
Author

timvw commented Apr 26, 2022

@wangfenjin Surprisingly I had started creating a similar flight sql client yesterday, threw it away, started from your WIP and updated the example client app..

Main areas that need attention:

  • make client generic again (currently switched to explicit tonic::transport::Channel)
  • action and update (commented them out)
  • iterating over streaming flightdata

@alamb
Copy link
Contributor

alamb commented Apr 27, 2022

I hope to find time to review this tomorrow

@HaoYang670
Copy link
Contributor

@alamb @viirya Could you please help to trigger the CI processes?

@viirya
Copy link
Member

viirya commented May 9, 2022

Triggered.

@codecov-commenter
Copy link

Codecov Report

Merging #1616 (202e0ec) into master (22e9f95) will decrease coverage by 0.22%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1616      +/-   ##
==========================================
- Coverage   83.02%   82.79%   -0.23%     
==========================================
  Files         193      194       +1     
  Lines       55612    55764     +152     
==========================================
- Hits        46174    46172       -2     
- Misses       9438     9592     +154     
Impacted Files Coverage Δ
arrow-flight/src/sql/client.rs 0.00% <0.00%> (ø)
arrow-flight/src/sql/mod.rs 0.00% <ø> (ø)
arrow-flight/src/sql/server.rs 0.00% <0.00%> (ø)
arrow/src/error.rs 21.56% <0.00%> (-2.88%) ⬇️
parquet_derive/src/parquet_field.rs 65.98% <0.00%> (-0.23%) ⬇️
parquet/src/encodings/encoding.rs 93.37% <0.00%> (-0.19%) ⬇️
arrow/src/array/transform/mod.rs 86.57% <0.00%> (-0.12%) ⬇️
arrow/src/datatypes/datatype.rs 66.80% <0.00%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22e9f95...202e0ec. Read the comment docs.

@alamb
Copy link
Contributor

alamb commented May 9, 2022

Thanks @timvw -- I am sorry I haven't spent much time helping to review this code. If you think it is ready to do so I'll try and give it a thorough review later this week

Thanks again

@wangfenjin
Copy link
Contributor

It's still WIP, maybe we can mark it as draft for now @timvw

@tustvold tustvold marked this pull request as draft May 19, 2022 15:04
@alamb
Copy link
Contributor

alamb commented Jul 21, 2022

cc @avantgardnerio here is some work related to making a FlightSQL client

@tustvold
Copy link
Contributor

Closing in favour of #3207

@tustvold tustvold closed this Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Basic FlightSQL Client
7 participants