Following up here, James Duong and Jose Almeida have submitted a set of pull requests proposing a set of additions to Flight SQL to expose more information about supported data types and provide metadata about column types in results. For anyone interested in reviewing the proposals, the pull requests can be found here: * https://github.com/apache/arrow/pull/11982 * https://github.com/apache/arrow/pull/11999 These PRs include implementations for C++ and Java as well as integration tests.
Thanks, David On Fri, Dec 17, 2021, at 17:07, James Duong wrote: > Yes, additional metadata would just be using the Field metadata map. The > protocol is the same, we have just pre-defined keys for some fields that > would be used for JDBC column attributes. > > Our preference would be that we get the currently approved protocol merged > into master first (after completing the integration tests) and then have a > separate vote on the TypeInfo changes. There's significant value in adding > Flight-SQL already and it'd be great to make that available. It's natural > that there will be an ongoing need to add extensions to the protocol as it > gets used in more scenarios. Now that we have a solid foundation, we can > examine further changes on a case-by-case basis. > > On Thu, Dec 16, 2021 at 2:42 PM David Li <lidav...@apache.org> wrote: > > > Strictly speaking we should have a vote since it is updating the format > > definition files we already voted on. > > > > I am curious about what exactly you mean by additional column metadata, > > but if it's just going to be encoded into the key-value metadata then I > > don't see a problem there. (As in: it sounds like it fits in the Field > > class given it's encoded in the Field metadata!) > > > > -David > > > > On Thu, Dec 16, 2021, at 16:14, James Duong wrote: > > > 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. > > > > > > > > -- > > *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. >
