Re: Making the NDV property required for theta sketch blobs in Puffin

Good point, the changes are contained to the theta sketch definition in this case. Nothing is changing in the Puffin metadata itself in this change. I missed that in the spec we already versioned the blob type for example: https://iceberg.apache.org/puffin-spec/#apache-datasketches-theta-v1-blob

Re: Making the NDV property required for theta sketch blobs in Puffin

Agreed this isn't a Puffin format change copying comment from the "Spec: Make NDV blob metadata property required" PR Puffin fole format has place for versioning within the magic, but the > Puffin format doesn't change, only its use changes. Puffin s

Re: Making the NDV property required for theta sketch blobs in Puffin

For the compatibility and version bump question, shouldn't this be a new revision of the theta sketch rather than a new version of Puffin? I think we want to avoid making changes to the file format itself if we can contain the changes to the sketch definitions. +1 for raising a vote once we have a

Re: Making the NDV property required for theta sketch blobs in Puffin

Hey all, Thanks everyone for providing your input on this discussion thread. There are 2 points I'd like to bring up, which in hindsight I should've recognized. If we look at the current Iceberg Improvement Process https://iceberg.apache.org/contribute/#apache-iceberg-improvement-proposals and

Re: Making the NDV property required for theta sketch blobs in Puffin

+1 for making the NDV property required in blob metadata Thanks, Steve Zhang > On Jun 21, 2024, at 2:54 PM, Amogh Jahagirdar <2am...@gmail.com> wrote: > > make the property required

Re: Making the NDV property required for theta sketch blobs in Puffin

+1, to make *apache-datasketches-theta-v1* sketch has the ndv blob property always present On Sat, Jun 22, 2024 at 3:20 AM Piotr Findeisen wrote: > Hi, > > non-binding +1 to require that `apache-datasketches-theta-v1` sketch has > ndv blob property set. > > Best > PF > > > On Sat, 22 Jun 2024 at

Re: Making the NDV property required for theta sketch blobs in Puffin

Hi, non-binding +1 to require that `apache-datasketches-theta-v1` sketch has ndv blob property set. Best PF On Sat, 22 Jun 2024 at 06:27, Jean-Baptiste Onofré wrote: > Hi Amogh > > +1 to have ndv blob metadata property required. > > As discussed during the last community meeting, we discussed

Re: Making the NDV property required for theta sketch blobs in Puffin

Hi Amogh +1 to have ndv blob metadata property required. As discussed during the last community meeting, we discussed voting on all code modification changes on spec. For the next spec changes, I would propose to start a voting thread, like "[VOTE][Puffin Spec] Make the ndv blob metadata property

Re: Making the NDV property required for theta sketch blobs in Puffin

I also support making this field required. Thanks, Nick Riasanovsky On Fri, Jun 21, 2024 at 7:00 PM Szehon Ho wrote: > It makes sense to me, normally changing optional -> required would > probably require a version bump, but maybe it is ok here as it is a > relatively new format, afaik adapted

Re: Making the NDV property required for theta sketch blobs in Puffin

It makes sense to me, normally changing optional -> required would probably require a version bump, but maybe it is ok here as it is a relatively new format, afaik adapted by Trino which already sets this field, but let's see if anyone disagrees. Thanks Szehon On Fri, Jun 21, 2024 at 3:35 PM huax

Re: Making the NDV property required for theta sketch blobs in Puffin

+1 for making the ndv blob metadata property required for theta sketches. On Fri, Jun 21, 2024 at 2:54 PM Amogh Jahagirdar <2am...@gmail.com> wrote: > Hey all, > > I wanted to raise this thread to discuss a spec change proposal > for making the ndv b