Skip to content

Implement Basic FlightSQL Client #1413#1616

Closed
timvw wants to merge 21 commits intoapache:masterfrom
timvw:flight_sql_client_example
Closed

Implement Basic FlightSQL Client #1413#1616
timvw wants to merge 21 commits intoapache:masterfrom
timvw:flight_sql_client_example

Conversation

@timvw
Copy link

@timvw timvw commented Apr 24, 2022 •
edited by jimexist
Loading

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
Merged
2 tasks
@wangfenjin
Copy link
Contributor

wangfenjin commented Apr 25, 2022

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

wangfenjin commented Apr 26, 2022

@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 •
edited
Loading

@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

wangfenjin
wangfenjin reviewed Apr 28, 2022
View reviewed changes
wangfenjin
wangfenjin reviewed Apr 28, 2022
View reviewed changes
wangfenjin and others added 19 commits May 1, 2022 06:13
@wangfenjin @timvw
WIP: impl flight sql client
0732f00 
Change-Id: I4cfc1412c05933c9c903d678a42462852d99fd28
@timvw
wip - starting from @wangfenjin wip
8cd531c
@timvw
changes to make at least Execute work
90aed27
@timvw
implemented getprimarykeys
23e79a6
@timvw
added some sample client outputs
49f75d1
@timvw
remove some todo
ac69391
@timvw
removed all todos
6360842
@timvw
make client generic again
9807916
@timvw
added command to execute an update query. currently fails on deserial…
4dceb7c 
…izing the putresult. not sure who's to blame for that
@timvw
added license header to exmaple client
1e480e7
@timvw
added licese header to client.md
ccedaba
@timvw
remove some unused imports
a3d4038
@timvw
remove unused variable
9a294fc
@timvw
undo making mod gen public. update example accordingly
b86953a
@timvw
cleanup some unused imports/cases
a7b785f
@timvw
ensure that build still works, even when flight-sql-experimental is n…
26f20af 
…ot turned on
@timvw
move client from examples to bin folder as flightsqlclient
0fdff30
@timvw
corrected arrow version to 13.0.0
33730b1
@timvw
Merge branch 'apache:master' into flight_sql_client_example
202e0ec
@HaoYang670
Copy link
Contributor

HaoYang670 commented May 9, 2022

@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-commenter commented May 9, 2022

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

wangfenjin commented May 9, 2022

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

tustvold commented Dec 19, 2022

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

Reviewers

1 more reviewer

@wangfenjin wangfenjin wangfenjin left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement Basic FlightSQL Client

7 participants

@timvw @wangfenjin @alamb @HaoYang670 @viirya @codecov-commenter @tustvold