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
