Conversation
There was a problem hiding this comment.
I'm thinking that a better test here would be to have a randomly generated batch/table with more field types, and more rows.
There was a problem hiding this comment.
Also, we'd ideally want JDBC/ODBC integration tests as well.
There was a problem hiding this comment.
Yeah, or reuse some of the datasets we have in the HTTP tests? IIRC there's one that tests JSON serialization for everything. Probably want to test some gnarlier datatypes like Parquet structs here too.
| // Use a new UUID to fingerprint a query | ||
| // TODO: Should we use something else here (and keep that in the results map)? |
There was a problem hiding this comment.
Is this how the caller references the query when asking us for a result? I think this should be a new random UUID every time (instead of being deterministic based on the original query), we won't care about caching at this level and I guess we also don't want to hold on to old query results in Seafowl for too long?
There was a problem hiding this comment.
That is correct, the client goes back with that "ticket" to collect the results.
Keeping it as UUID's then.
| use futures::{future::join_all, Future, FutureExt}; | ||
|
|
||
| use pretty_env_logger::env_logger; | ||
| use seafowl::frontend::flight::run_flight_server; |
There was a problem hiding this comment.
shouldn't this be behind a cfg(feature = ...)?
src/frontend/flight/sql.rs
Outdated
|
|
||
| let flight_info = FlightInfo::new() | ||
| .try_with_schema(&schema) | ||
| .expect("encoding schema") |
There was a problem hiding this comment.
Why is this an expect instead of propagating the error up?
There was a problem hiding this comment.
Oh good catch, forgot to transform that.
There was a problem hiding this comment.
Yeah, or reuse some of the datasets we have in the HTTP tests? IIRC there's one that tests JSON serialization for everything. Probably want to test some gnarlier datatypes like Parquet structs here too.
tests/flight/client.rs
Outdated
|
|
||
| // Try out the actual query | ||
| let cmd = CommandStatementQuery { | ||
| query: "SELECT * FROM flight_table".to_string(), |
There was a problem hiding this comment.
I'd test some basic DDL here (CREATE TABLE / CREATE EXTERNAL TABLE maybe) and a slightly more complex SELECT. Maybe also test two queries getting executed at the same time, with interleaving? i.e. start Q1, start Q2, get results for Q2, get results for Q1.
gruuya
force-pushed
the
arrow-flight-frontend
branch
from
60e42d9 to
f251b51
Compare
2 participants
Closes #477.
This PR introduces an optional Arrow Flight SQL interface to Seafowl.