Should we merge https://github.com/apache/parquet-format/pull/474 before releasing the format?
On Thu, Mar 20, 2025 at 12:52 AM Fokko Driesprong <fo...@apache.org> wrote: > As mentioned earlier, it looks like the implementations > <https://lists.apache.org/thread/361035vf227w11q6r6w6t67618jlpfbx> are > moving forward. I think it would be good to have the format released, as > long as we're all comfortable with the current state to unblock the other > releases. > > Kind regards, > Fokko > > > > Op wo 19 mrt 2025 om 04:15 schreef Gang Wu <ust...@gmail.com>: > > > I'm fine with releasing the parquet-format to unblock variant (and > > geometry) development. > > > > We can add a statement to the documentation and release note to warn that > > this is experimental and subject to change. > > > > Best, > > Gang > > > > On Wed, Mar 19, 2025 at 7:08 AM Aihua Xu <aihu...@gmail.com> wrote: > > > > > Hi Ryan, > > > > > > Thanks a lot for explaining the process to add Variant type annotation > in > > > Java. It would be great if we can add the logical type now to unblock > > some > > > code paths which rely on such logical types. > > > > > > On Tue, Mar 18, 2025 at 1:22 PM Ryan Blue <rdb...@gmail.com> wrote: > > > > > > > Hi everyone, > > > > Last community sync, we had a few Variant topics that I want to also > > > raise > > > > here. > > > > > > > > First, I want to highlight that we talked about spec versioning and > > there > > > > was agreement to avoid unnecessary complexity by maintaining one > > version > > > > that covers both encoding and shredding. The two parts of Variant are > > in > > > > separate markdown docs, but we don’t want to have fragmentation where > > an > > > > implementation follows the encoding but not shredding. We want to > keep > > > the > > > > two fully compatible and ensure that you can rely on shredding for > data > > > > skipping. Note that this doesn’t add many requirements for writers > > > > (shredding is optional), but ensures that shredding can be used by > > > > requiring support in readers. This wasn’t very controversial, but > > please > > > > reply if you have concerns about it. > > > > > > > > Second, in the sync I also brought up the need to get the Variant > type > > > > annotation into a release. There was some pushback on this, I think, > > > > because non-Java projects have a simpler build process. I don’t think > > > many > > > > people realized how the Java side works: > > > > > > > > - The thrift file is released in the parquet-format Jar > > > > - The parquet-java maven build has a parquet-format-structures > > module > > > > that downloads the parquet-format Jar and generates Java classes > > from > > > > the > > > > Thrift definition > > > > - Then there is a translation step in ParquetMetadataConverter > that > > > > converts to Parquet (rather than Thrift) classes > > > > - The Parquet API adds utilities to use the converted metadata > > > classes, > > > > like LogicalTypeAnnotationVisitor > > > > > > > > We discussed creating sample files to verify Variant compatibility, > but > > > in > > > > order to produce those files from the Variant implementation I’m > > > building, > > > > I would need to produce a one-off version of the thrift definition, > > > inject > > > > it into the parquet-thrift-format-structures build, update the > Parquet > > > API > > > > to add a VariantLogicalTypeAnnotation, and then find a way to get > that > > > > temporary build into a Maven repository so that PRs can depend on it. > > > Those > > > > PRs can’t be merged because they would rely on an > unpublished/snapshot > > > Jar. > > > > > > > > This is becoming a big headache and I don’t think that it helps us > > > deliver > > > > Variant reliably. We know that we are going to support Variant data > and > > > the > > > > type annotation to label that data is going to be needed. There is > > little > > > > risk in committing that annotation now. The argument against adding > it > > > now > > > > was that it may create confusion, but I think it is unlikely that > > anyone > > > > would see the annotation in parquet-java or parquet-format and assume > > > > support guarantees. The spec is clearly marked experimental and the > > > > annotation is referenced already by LogicalTypes.md > > > > < > > > > > > > > > > https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#variant > > > > >. > > > > Adding the annotation to the API doesn’t significantly increase the > > > number > > > > of people seeing it, and the people that do are already working with > > > > Parquet internals. > > > > > > > > The only other objection I know about is that we haven’t yet > finalized > > > what > > > > goes in the annotation, which I agree is a prerequisite for moving > > > forward. > > > > I suggest that we add the encoding/shredding version (1), release > > > > parquet-format, and start working on the API extensions in > > parquet-java. > > > > Then we can actually work on a Java implementation that stores data > in > > > > files in the parquet-java project. > > > > > > > > Does that sound reasonable? > > > > > > > > Ryan > > > > > > > > > >