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

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Timur Magomedov <t(dot)magomedov(at)postgrespro(dot)ru>
Cc: Tomas Vondra <tomas(at)vondra(dot)me>, "Aya Iwata (Fujitsu)" <iwata(dot)aya(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [WIP]Vertical Clustered Index (columnar store extension) - take2
Date: 2025-07-01 03:17:54
Message-ID: CAHut+Pvrf9tWO9X3img0Eca35_zsc-ESxCp5HSqCxZfmAm4hbA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2025-07-01 03:20:07 implicit casts from void*
Previous Message Richard Guo 2025-07-01 02:57:44 Re: Pathify RHS unique-ification for semijoin planning