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

On Tue, Aug 12, 2025 at 1:48 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
...
> 1.
> PFA the other typos.
>

Fixed in v18.

> 2.
> I found it skip vci query context initialization in vci_intialize_query_context()
> if full page writes is disabled, Could you explain why we need full page write
> enabled for VCI?
>

TODO

> 3.
> Both vci_ros.h and vci_ros.c have a comment about accessing the VCI main relation
> header, but they are slightly different. Could we sync them and keep only one?
>
> It seems the comment is outdated, as the functions vci_KeepReadingMainRelHeader()
> and vci_KeepWritingMainRelHeader() do not exist in the current VCI implementation.
>

TODO. I agree that there should be only one comment, and outdated
function references need to be fixed.

> 4.
> +/**
> + * This function is assumed when the VCI index is newly built, and
> + * it converts all the data in the relation of PostgreSQL into ROS.
> + */
> +uint64
> +vci_ConvertWos2RosForBuild(Relation mainRel,
> + Size workareaSize,
> + IndexInfo *indexInfo)
> ...
> + result = (uint64) table_index_build_scan(comContext.heapRel,
> + mainRel,
> + indexInfo,
> + true, /* allow syncscan */
> + true,
> + vci_build_callback,
> + (void *) &comContext, NULL)
>
> Perhaps we can use a double return type to avoid type casting since
> other places also use double.

Fixed in v18.

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