From: | Timur Magomedov <t(dot)magomedov(at)postgrespro(dot)ru> |
---|---|
To: | Peter Smith <smithpb2250(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me> |
Cc: | "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-06-04 13:47:31 |
Message-ID: | c4e314ef0a58050c8b7847ac1852555876674a69.camel@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
0002-Meson-fixes.patch
Fixed some ghost files in Meson build, added VCI to
contrib/meson.build.
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.
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.
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".
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.
--
Regards,
Timur Magomedov
Attachment | Content-Type | Size |
---|---|---|
0001-Removed-unnecessary-preload-libraries-checks.patch | text/x-patch | 3.0 KB |
0002-Meson-fixes.patch | text/x-patch | 2.2 KB |
0003-Removed-unused-OS-specific-CAS-functions.patch | text/x-patch | 3.6 KB |
0004-Fixed-segfault-in-case-there-is-no-hottable.patch | text/x-patch | 791 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | * Neustradamus * | 2025-06-04 13:47:51 | Re: Please update your contact list: It must be pgsql-hackers@lists.postgresql.org |
Previous Message | Nathan Bossart | 2025-06-04 13:47:00 | Re: [PATCH] Hex-coding optimizations using SVE on ARM. |