Thanks all for the responses. Ryan, you are correct, as we should only have a binary dependency. Right now we're compiling the source of Thrift in the CI.
Of course, I can add some automated tests to read/write different versions of Thrift. Afaik it should be binary compatible, both forward as backward. So these tests might not add any value. Cheers, Fokko Op ma 28 jan. 2019 om 09:03 schreef Gabor Szadovszky <[email protected]>: > I've also written some parquet files with Fokko's patch and checked by > using older releases of parquet-tools. Worked just fine. > > On Sun, Jan 27, 2019 at 10:14 PM Uwe L. Korn <[email protected]> wrote: > > > To give a bit of input: I have written a simple Parquet file with > > pyarrow/parquet-cpp, one with Thrift 0.11, the other with Thrift 0.12 and > > they were identical. We have been using Thrift 0.11 for quite some while > in > > there, so I would assume that there are no problems between the version. > > Otherwise I would have expected users reporting many issues about > > pyarrow/Spark interop. > > > > Uwe > > > > On Sun, Jan 27, 2019, at 3:33 AM, Wes McKinney wrote: > > > I thought the Thrift binary protocol is stable at this point (I'm not > > > sure what process Apache Thrift uses to ensure this), but I agree it > > > would be worth a smoke test of new-Thrift vs. old-Thrift. I've build > > > parquet-cpp with 0.12 and not had any issues > > > > > > - Wes > > > > > > On Fri, Jan 25, 2019 at 10:34 AM Ryan Blue <[email protected]> > > wrote: > > > > > > > > The thrift dependency in parquet-format and the one in parquet-mr can > > > > coexist because we shade the one in parquet-format. Thrift should > also > > be > > > > binary compatible, although I don't think they publish any > guarantees. > > > > > > > > On Fri, Jan 25, 2019 at 12:53 AM Uwe L. Korn <[email protected]> > wrote: > > > > > > > > > As an FYI: parquet-cpp already uses Thrift 0.12 in some of its > binary > > > > > distributions. So when there is a problem with old readers, one has > > to > > > > > notice that we already have files out in the wild. > > > > > > > > > > Cheers > > > > > Uwe > > > > > > > > > > On Fri, Jan 25, 2019, at 9:13 AM, Gabor Szadovszky wrote: > > > > > > May it cause any problems that we write the thrift structures in > > the > > > > > > parquet files (footer, page headers etc.) with a different > version > > as > > > > > > before? It might require some tests if the older readers are able > > to read > > > > > > the files written with the new thrift. > > > > > > Any thoughts? > > > > > > > > > > > > On Thu, Jan 24, 2019 at 8:49 PM Ryan Blue > > <[email protected]> > > > > > wrote: > > > > > > > > > > > > > Why is it a problem that thrift can't be compiled with Java 11? > > We > > > > > should > > > > > > > only have a binary dependency. > > > > > > > > > > > > > > +1 for moving thrift forward, though. > > > > > > > > > > > > > > On Thu, Jan 24, 2019 at 11:38 AM Driesprong, Fokko > > > > > <[email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > I would like to discuss updating the Thrift dependency to > > 0.12.0 of > > > > > > > > Parquet. In my effort to make Parquet forward compatible for > > JDK11 > > > > > > > > <https://github.com/apache/parquet-mr/pull/596>, I stumbled > > upon > > > > > some > > > > > > > > issues. One of them that we still rely, in both the CI and > > > > > documentation, > > > > > > > > on Thrift 0.9.3 (released October 2015). Unfortunately, this > > version > > > > > of > > > > > > > > Thrift won't compile under Java11: > > > > > > > > [javac] Compiling 105 source files to > > > > > > > /home/travis/build/apache/parquet-mr > > > > > > > > /thrift-0.9.3/lib/java/build > > > > > > > > [javac] warning: [options] bootstrap class path not set in > > > > > conjunction > > > > > > > with > > > > > > > > -source 5 > > > > > > > > [javac] error: Source option 5 is no longer supported. Use 6 > or > > > > > later. > > > > > > > > [javac] error: Target option 1.5 is no longer supported. Use > > 1.6 or > > > > > > > later. > > > > > > > > > > > > > > > > Target 1.5, feels a bit awkward, doesn't it? My main question > > to the > > > > > > > > dev-list is; is there any particular reason why we shouldn't > > update > > > > > the > > > > > > > > Thrift dependency to 0.12.0. I know that it will have an > > impact on > > > > > > > Parquet, > > > > > > > > but if we want to support Java11, we need to move forward > > eventually. > > > > > > > > > > > > > > > > After updating the thrift-maven plugin > > > > > > > > <https://github.com/apache/parquet-mr/pull/600>, I was able > > to run > > > > > the > > > > > > > CI > > > > > > > > against Thrift 0.12.0 < > > https://github.com/apache/parquet-mr/pull/601 > > > > > >. > > > > > > > > > > > > > > > > Cheers, Fokko > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Ryan Blue > > > > > > > Software Engineer > > > > > > > Netflix > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Ryan Blue > > > > Software Engineer > > > > Netflix > > >
