Re: [WIP]Vertical Clustered Index (columnar store extension) - take2

On Tue, Jul 1, 2025 at 1:17 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Sat, Jun 28, 2025 at 12:24 AM Timur Magomedov
> <t(dot)magomedov(at)postgrespro(dot)ru> wrote:
> ...
> > Thanks for updates.
> > I was trying to move some code from Postgres core patch to contrib/vci.
> > Started with moving VCI options from StdRdOptions struct and
> > reloptions.c, reloptions.h files into contrib/vci like Tomas suggested
> > earlier. Getting new relopt kind using add_reloption_kind() instead of
> > hardcoded RELOPT_KIND_VCI and so on, just like in contrib/bloom.
> >
> > During this work I've found that those VCI-specific options,
> > "vci_column_ids" and "vci_dropped_column_ids", are only read in case
> > vci_IsExtendedToMoreThan32Columns() is true. And
> > vci_IsExtendedToMoreThan32Columns() itself checks if vci_column_ids
> > option is non-empty string to return true. AFAIK no one sets those
> > options and vci_IsExtendedToMoreThan32Columns() always returns false.
> >
> > There are comments in code mentioning that one can create VCI index
> > with more than 32 columns using vci_create(). However, there is no such
> > a function anywhere. I guess it is now impossible to create VCI index
> > with more than 32 (INDEX_MAX_KEYS) columns. Maybe the options were used
> > to store column information for indexes created with vci_create() but
> > this function has gone.
> >
> > So, giving the ideas above, can we remove those vci_column_ids,
> > vci_dropped_column_ids options and all the code that assumes
> > vci_IsExtendedToMoreThan32Columns() to be true? This would make core
> > patch a bit shorter, specifically there will be no changes in
> > reloptions.{c,h} files and no changes in StdRdOptions struct in rel.h.
> >
>
> Hi Timur.
>
> Thanks for reporting this! Your idea seems correct to me.
>
> AFAICT, those relops were intended for supporting >32 vci indexes,
> which is only possible when the VCI index is created using
> “vci_create()” function, but that function does not exist in the VCI
> implementation posted to OSS. So, I agree with you – since those
> relops are not currently needed they can just be removed. I will
> confirm this understanding with the original patch devs and, if
> correct, I will address this in a future patch version.
>

Fixed in v10.

======
Kind Regards,
Peter Smith.
Fujitsu Australia.