Horton

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

Peter Smith <smithpb2250@gmail.com>

View thread
From: Peter Smith <smithpb2250@gmail.com>
To: Japin Li <japinli@hotmail.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas@vondra.me>, "Aya Iwata (Fujitsu)" <iwata.aya@fujitsu.com>, Timur Magomedov <t.magomedov@postgrespro.ru>, shveta malik <shveta.malik@gmail.com>
Date: 2025-08-14T01:39:21Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Mark ItemPointer arguments as const throughout

  2. Reorganize GUC structs

  3. Eliminate XLOG_HEAP2_VISIBLE from vacuum phase III

  4. Update various forward declarations to use typedef

  5. Pathify RHS unique-ification for semijoin planning

  6. Revert "Don't lock partitions pruned by initial pruning"

  7. For inplace update, send nontransactional invalidations.

  8. Remove nearly-unused SizeOfIptrData macro.

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