Hi David, While working on the JDBC driver on top of Flight SQL and on integration tests, we identified a couple of enhancements that were needed. 1. The ability to report data type information, as done in this PR: https://github.com/apache/arrow/pull/11982. This PR adds another RPC request for this information. 2. Additional column metadata that's outside of the Schema/Field classes in Arrow (PR pending) when returning Arrow schemas. The planned PR uses the Arrow Field's MetadataMap to encode extra metadata rather than altering any protobuf definitions.
Should these additional changes go in together with the rest of Flight-SQL, or be approved separately? On Thu, Dec 16, 2021 at 7:54 AM Kyle Porter <ky...@bitquilltech.com.invalid> wrote: > Thanks All - we'll look to get the tests merged into this branch so we can > close ASAP. > > *Kyle Porter* > CEO > Bit Quill Technologies Inc. > Office: +1.778.331.3355 | Direct: +1.604.441.7318 | ky...@bitquilltech.com > https://www.bitquill.com > > This email message is for the sole use of the intended recipient(s) and may > contain confidential and privileged information. Any unauthorized review, > use, disclosure, or distribution is prohibited. If you are not the > intended recipient, please contact the sender by reply email and destroy > all copies of the original message. Thank you. > > > On Wed, Dec 15, 2021 at 9:11 AM David Li <lidav...@apache.org> wrote: > > > My vote: +1 > > > > The vote passes with three +1 (binding) votes, one +1 (non binding) vote, > > and one -0.5 (binding) vote. > > > > However, we will first merge into a separate branch and implement > > integration tests before merging into the main branch. JIRA for > integration > > tests: https://issues.apache.org/jira/browse/ARROW-15112 > > > > @Kyle I've created the branch flight-sql[1], would you prefer I merge in > > your existing PRs, or would you prefer to create new PRs against that > > branch (given you've already started on things)? > > > > On a side note - do we document the requirements for proposed additions > > somewhere? (multiple implementations, integration tests) It would be nice > > to have it on hand for reference. > > > > [1]: https://github.com/apache/arrow/tree/flight-sql > > > > -David > > > > On Mon, Dec 13, 2021, at 11:25, Kyle Porter wrote: > > > Thanks David, > > > > > > Yes, the team is actually already looking at adding the cross language > > > tests apologies for not communicating that earlier > > > > > > On Mon., Dec. 13, 2021, 12:18 p.m. David Li, <lidav...@apache.org> > > wrote: > > > > > > > Are any other PMC members able to look at this? > > > > > > > > > > > OK by me. We could also create a branch to merge the PRs add > the > > > > > > > integration tests, and then merge all at once. > > > > > > > > Kyle, is this an ok solution? Would you & your team be able to get > > > > integration tests done reasonably soon? > > > > > > > > There's some setup for Flight integration tests already: > > > > > > > https://github.com/apache/arrow/blob/11be9c542b9699b7eb4ae1656775c9b30639e415/dev/archery/archery/integration/runner.py#L375-L385 > > > > > > > > So what would be needed are: > > > > > > > > 1. Enable Flight SQL for the integration test container > > > > 2. Link the integration test client/server to Flight SQL > > > > 3. Add one or more test scenarios in the integration test runner, and > > in > > > > the integration test client/server > > > > > > > > It might be acceptable to just hardcode expected requests/responses > > > > instead of integrating SQLite/Derby (as was done for the individual > > > > language tests) since the focus should be on just the protocol and > not > > > > particular implementations. > > > > > > > > -David > > > > > > > > On Sun, Dec 12, 2021, at 16:21, Wes McKinney wrote: > > > > > +1. Agree re: adding integration tests as soon as practical > > > > > > > > > > On Thu, Dec 9, 2021 at 5:21 AM Ravindra Pindikura < > > ravin...@dremio.com> > > > > wrote: > > > > > > > > > > > > +1 > > > > > > > > > > > > On Wed, Dec 8, 2021 at 11:42 PM Micah Kornfield < > > emkornfi...@gmail.com > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Given that the C++ and Java components are in separate PRs, > > would > > > > it be > > > > > > > > acceptable to add after the initial merge? > > > > > > > > > > > > > > > > > > > > > OK by me. We could also create a branch to merge the PRs add > the > > > > > > > integration tests, and then merge all at once. > > > > > > > > > > > > > > On Wed, Dec 8, 2021 at 10:07 AM Kyle Porter < > > ky...@bitquilltech.com> > > > > > > > wrote: > > > > > > > > > > > > > > > Given that the C++ and Java components are in separate PRs, > > would > > > > it be > > > > > > > > acceptable to add after the initial merge? > > > > > > > > > > > > > > > > *Kyle Porter* > > > > > > > > CEO > > > > > > > > Bit Quill Technologies Inc. > > > > > > > > Office: +1.778.331.3355 | Direct: +1.604.441.7318 | > > > > > > > ky...@bitquilltech.com > > > > > > > > https://www.bitquill.com > > > > > > > > > > > > > > > > This email message is for the sole use of the intended > > > > recipient(s) and > > > > > > > > may contain confidential and privileged information. Any > > > > unauthorized > > > > > > > > review, use, disclosure, or distribution is prohibited. If > you > > > > are not > > > > > > > the > > > > > > > > intended recipient, please contact the sender by reply email > > and > > > > destroy > > > > > > > > all copies of the original message. Thank you. > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 8, 2021 at 2:03 PM Micah Kornfield < > > > > emkornfi...@gmail.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > >> > > > > > > > > >> > There is not an integration test. Do we want to require > > this? > > > > > > > >> > > > > > > > >> It would be nice, I'm -0.5 vote without one. So if enough > > PMC > > > > members > > > > > > > >> want to forgo the integration test the vote can still pass. > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > Is cross language testing something that's usually done? > > > > > > > >> > > > > > > > >> Yes. One of the value propositions of Arrow is the > > cross-language > > > > > > > >> support. The community agreed to specification changes > (and I > > > > assumed > > > > > > > >> this > > > > > > > >> covers new specifications) need to have reference > > implementations > > > > in > > > > > > > >> C++/Java with integration testing between the two. > > > > > > > >> > > > > > > > >> On Wed, Dec 8, 2021 at 5:21 AM Kyle Porter < > > > > ky...@bitquilltech.com > > > > > > > >> .invalid> > > > > > > > >> wrote: > > > > > > > >> > > > > > > > >> > The team initially developed the C++ client against the > Java > > > > server, > > > > > > > and > > > > > > > >> > have done some cross language testing. It wasn't > exhaustive > > or > > > > > > > >> methodical > > > > > > > >> > in nature, however. Is cross language testing something > > that's > > > > usually > > > > > > > >> > done? > > > > > > > >> > > > > > > > > >> > On Wed., Dec. 8, 2021, 9:18 a.m. David Li, < > > lidav...@apache.org > > > > > > > > > > > > wrote: > > > > > > > >> > > > > > > > > >> > > There is not an integration test. Do we want to require > > this? > > > > > > > >> > > > > > > > > > >> > > Also CC @Kyle, in case your team has done such testing. > > > > > > > >> > > > > > > > > > >> > > It looks like Flight itself did not have a test for a > few > > > > versions > > > > > > > >> after > > > > > > > >> > > it was initially implemented. > > > > > > > >> > > > > > > > > > >> > > -David > > > > > > > >> > > > > > > > > > >> > > On Tue, Dec 7, 2021, at 23:19, Micah Kornfield wrote: > > > > > > > >> > > > Is there an integration test between the two > languages? > > > > > > > >> > > > > > > > > > > >> > > > On Tue, Dec 7, 2021 at 1:35 PM David Li < > > > > lidav...@apache.org> > > > > > > > >> wrote: > > > > > > > >> > > > > > > > > > > >> > > > > Hello, > > > > > > > >> > > > > > > > > > > > >> > > > > Kyle Porter, Rafael Telles, Ryan Nicholson, et. al. > > have > > > > > > > proposed > > > > > > > >> > > adding > > > > > > > >> > > > > Arrow Flight SQL, an experimental protocol for > > > > interacting with > > > > > > > >> SQL > > > > > > > >> > > > > databases over Arrow Flight [1], as explained in a > > > > previous ML > > > > > > > >> > > discussion > > > > > > > >> > > > > [2] and in a design document [3]. The purpose of > > Flight > > > > SQL is > > > > > > > to > > > > > > > >> > allow > > > > > > > >> > > > > clients and SQL database servers to communicate > > (execute > > > > > > > queries, > > > > > > > >> > list > > > > > > > >> > > > > tables, create prepared statements, etc.) using > Arrow > > and > > > > Arrow > > > > > > > >> > > Flight, by > > > > > > > >> > > > > defining how to use Flight RPC methods, as well as > > message > > > > > > > >> payloads > > > > > > > >> > to > > > > > > > >> > > use > > > > > > > >> > > > > with those methods. > > > > > > > >> > > > > > > > > > > > >> > > > > The new protocol definitions can be found at [4]. > > > > > > > >> > > > > > > > > > > > >> > > > > They have provided pull requests implementing the > > server > > > > and > > > > > > > >> client > > > > > > > >> > > > > protocol in C++ [5] and Java [6] which can be merged > > > > after this > > > > > > > >> > > addition is > > > > > > > >> > > > > approved. > > > > > > > >> > > > > > > > > > > > >> > > > > Please vote whether to accept this addition. The > vote > > > > will be > > > > > > > open > > > > > > > >> > for > > > > > > > >> > > at > > > > > > > >> > > > > least 72 hours. > > > > > > > >> > > > > > > > > > > > >> > > > > [1]: > https://arrow.apache.org/docs/format/Flight.html > > > > > > > >> > > > > [2]: > > > > > > > >> > > > > > https://lists.apache.org/thread/s08b20ty756qq10zybd9qr0mm4jhmz93 > > > > > > > >> > > > > [3]: > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > https://docs.google.com/document/d/1WQz32bDF06GgMdEYyzhakqUigBZkALFwDF2y1x3DTAI/edit?usp=sharing > > > > > > > >> > > > > Note that the protocol definitions in the design > > document > > > > are > > > > > > > out > > > > > > > >> of > > > > > > > >> > > date; > > > > > > > >> > > > > the canonical reference is in the pull requests. > > > > > > > >> > > > > [4]: > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > https://github.com/apache/arrow/blob/72ce72ba855909052f7dfb898105b419697157c8/format/FlightSql.proto > > > > > > > >> > > > > [5]: https://github.com/apache/arrow/pull/11507 > > > > > > > >> > > > > [6]: https://github.com/apache/arrow/pull/10906 > > > > > > > >> > > > > > > > > > > > >> > > > > Thanks, > > > > > > > >> > > > > David > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Thanks and regards, > > > > > > Ravindra. > > > > > > > > > > > > > > > -- *James Duong* Lead Software Developer Bit Quill Technologies Inc. Direct: +1.604.562.6082 | jam...@bitquilltech.com https://www.bitquilltech.com This email message is for the sole use of the intended recipient(s) and may contain confidential and privileged information. Any unauthorized review, use, disclosure, or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. Thank you.
