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

Hi Timur.

On Wed, Jun 4, 2025 at 11:47 PM Timur Magomedov
<t.magomedov@postgrespro.ru> wrote:
>
> Hello Peter,
>
> I've tried running VCI, the idea looks great to me. Thank you and
> Iwata-san for working on this feature. Looked at the source code of VCI
> module, the patch is huge but here are some ideas I hope could be
> useful for community. Made some patches on top of v4 that can be
> squashed into future iteration of VCI module.
>
> 0001-Removed-unnecessary-preload-libraries-checks.patch
> Please correct me if I'm wrong but there is no need for adding VCI to
> "session_preload_libraries" in case it is in "shared_preload_libraries"
> already.

I think you are correct, but I am still checking the history of this
code before making this change.

>
> 0002-Meson-fixes.patch
> Fixed some ghost files in Meson build, added VCI to
> contrib/meson.build.

Done.

> 0003-Removed-unused-OS-specific-CAS-functions.patch
> Having own VCI-specific CAS looks scary to me, fortunately looks like
> it was unused and can be removed.

Done.

>
> 0004-Fixed-segfault-in-case-there-is-no-hottable.patch
> The most exciting one, faced it while playing around with queries from
> ClickBench benchmark. Segfault was caused by accessing
> aggstate->hottable even in case aggstate->hottable is NULL, using
> aggstate->hashtable instead fixes this.

Done.

>
> Also there are some ideas of making the VCI module patch smaller and
> simpler for review:
>
> First of all, we have this hottable optimization which is enabled but
> surrounded by "#ifdef VCI_ENABLE_HOT_HASH_TABLE ... #endif". I think
> this could be a separate patch on top of VCI module patch, say "Hot
> hashtable optimization".

Done.

> Second, there is some OS-specific code that looks like used only in
> case ENABLE_WOSROS_TRANS_DEFERRAL is defined. This feature is unclear
> to me, probably it can be better reviewed if separated to another
> "Deferral of WOS->ROS transforamtion" patch that would introduce new
> GUC and some OS-specific code. So we can be sure other VCI code has no
> OS-specific parts.
>

Done.

~~~

PSA v5 patches. These incorporate most of Timur's top-up patches and
patch splitting ideas [1],  plus other minor changes:

v5-0001
- same as v4-0001

v5-0002
- Lots of comment prefixes are modified -- e.g. "/**<" to "/*"
- Includes top-up patch 0002 (meson fixes)
- Includes top-up patch 0003 (removes some unused OS-specific functions)
- Includes top-up patch 0004 (fixed segfault)

v5-0003
All the VCI_ENABLE_HOT_HASH_TABLE code is moved from the previous patch 0002.

v5-0004
All the ENABLE_WOSROS_TRANS_DEFERAL code is moved from the previous patch 0002.

v5-0003
- same as v4-0003

======
[1] https://www.postgresql.org/message-id/c4e314ef0a58050c8b7847ac1852555876674a69.camel%40postgrespro.ru

Kind Regards,
Peter Smith.
Fujitsu Australia