From: | Japin Li <japinli(at)hotmail(dot)com> |
---|---|
To: | Timur Magomedov <t(dot)magomedov(at)postgrespro(dot)ru> |
Cc: | Peter Smith <smithpb2250(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tomas Vondra <tomas(at)vondra(dot)me>, "Aya Iwata (Fujitsu)" <iwata(dot)aya(at)fujitsu(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
Subject: | Re: [WIP]Vertical Clustered Index (columnar store extension) - take2 |
Date: | 2025-08-14 02:08:35 |
Message-ID: | ME0P300MB044529CCB8420B12DB15567BB635A@ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Aug 13, 2025 at 07:28:34PM +0300, Timur Magomedov 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.
>
Yeah. The code isn't used in these patches, so it's a good idea to remove it.
--
Best regards,
Japin Li
ChengDu WenWu Information Technology Co., LTD.
From | Date | Subject | |
---|---|---|---|
Next Message | Japin Li | 2025-08-14 02:11:07 | Re: [WIP]Vertical Clustered Index (columnar store extension) - take2 |
Previous Message | Peter Smith | 2025-08-14 02:04:12 | Re: [WIP]Vertical Clustered Index (columnar store extension) - take2 |