Horton

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

Peter Smith <smithpb2250@gmail.com>

View thread
From: Peter Smith <smithpb2250@gmail.com>
To: Timur Magomedov <t.magomedov@postgrespro.ru>
Cc: Japin Li <japinli@hotmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas@vondra.me>, "Aya Iwata (Fujitsu)" <iwata.aya@fujitsu.com>, shveta malik <shveta.malik@gmail.com>
Date: 2025-08-14T01:44:56Z
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 Thu, Aug 14, 2025 at 2:28 AM Timur Magomedov
<t.magomedov@postgrespro.ru> wrote:
>
> On Mon, 2025-08-11 at 17:39 +0800, Japin Li wrote:
> >
> > 1.
> > +static struct
> > +{
> > +    int         transfn_oid;    /* Transition function's funcoid.
> > Arrays are
> > +                                 * sorted in ascending order */
> > +    Oid         transtype;      /* Transition data type */
> > +    PGFunction  merge_trans;    /* Function pointer set required for
> > parallel
> > +                                 * aggregation for each transfn_oid
> > */
> > +    vci_aggtranstype_kind kind; /* If transtype is INTERNALOID, its
> > details */
> > +}           trans_funcs_table[] = {
> > +    {F_FLOAT4_ACCUM, 1022, merge_floatX_accum,
> > VCI_AGG_NOT_INTERNAL},   /* 208 */
> > +    {F_FLOAT8_ACCUM, 1022, merge_floatX_accum,
> > VCI_AGG_NOT_INTERNAL},   /* 222 */
> > +    {F_INT8INC, 20, int8pl, VCI_AGG_NOT_INTERNAL},  /* 1833 */
> > +    {F_NUMERIC_ACCUM, 2281, numeric_combine,
> > VCI_AGG_NUMERIC_AGG_STATE},    /* 1834 */
> > +    {F_INT2_ACCUM, 2281, numeric_poly_combine,
> > VCI_AGG_POLY_NUM_AGG_STATE}, /* 1836 */
> > +    {F_INT4_ACCUM, 2281, numeric_poly_combine,
> > VCI_AGG_POLY_NUM_AGG_STATE}, /* 1835 */
> > +    {F_INT8_ACCUM, 2281, numeric_combine,
> > VCI_AGG_NUMERIC_AGG_STATE},   /* 1836 */
> > +    {F_INT2_SUM, 20, int8pl, VCI_AGG_NOT_INTERNAL}, /* 1840 */
> > +    {F_INT4_SUM, 20, int8pl, VCI_AGG_NOT_INTERNAL}, /* 1841 */
> > +    {F_INTERVAL_AVG_COMBINE, 2281, merge_interval_avg_accum,
> > VCI_AGG_NOT_INTERNAL}, /* 3325 */
> > +    {F_INT2_AVG_ACCUM, 1016, merge_intX_accum,
> > VCI_AGG_NOT_INTERNAL},   /* 1962 */
> > +    {F_INT4_AVG_ACCUM, 1016, merge_intX_accum,
> > VCI_AGG_NOT_INTERNAL},   /* 1963 */
> > +    {F_INT8INC_ANY, 20, int8pl, VCI_AGG_NOT_INTERNAL},  /* 2804 */
> > +    {F_INT8_AVG_ACCUM, 2281, int8_avg_combine,
> > VCI_AGG_POLY_AVG_NUM_AGG_STATE}, /* 2746 */
> > +    {F_NUMERIC_AVG_ACCUM, 2281, numeric_avg_combine,
> > VCI_AGG_AVG_NUMERIC_AGG_STATE},    /* 2858 */
> > +};
> >
> > The comments state that this is sorted in ascending order, but the
> > code doesn't
> > follow that rule. While the current linear search works, a future
> > change to
> > binary search could cause problems.
> >
>
> Hi Japin!
> I've looked at the code, vci_set_merge_and_copy_trans_funcs() function
> is unused and almost all code of vci_aggmergetranstype.c file including
> trans_funcs_table[] can be either removed either replaced with simple
> switch-case. Only transfn_oid fields of trans_funcs_table[] were
> actually used. Here is my patch made on top of v17.
>
> Peter, what do you think? Is it OK to remove those code?
>

Hi Timur. Thanks for the suggestion/patch.

I had already prepared the v18 patches using Japin's reordering of
that array;  I'll study your switch/removal patch for possible
inclusion in v19 (next week?).

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