On Tue, Aug 12, 2025 at 1:48 PM Japin Li <japinli@hotmail.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