Re: [WIP]Vertical Clustered Index (columnar store extension) - take2
Timur Magomedov <t.magomedov@postgrespro.ru>
From: Timur Magomedov <t.magomedov@postgrespro.ru>
To: Japin Li <japinli@hotmail.com>, Peter Smith <smithpb2250@gmail.com>
Cc: "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-13T16:28:34Z
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 →
-
Mark ItemPointer arguments as const throughout
- e1ac846f3d28 19 (unreleased) cited
-
Reorganize GUC structs
- a13833c35f9e 19 (unreleased) cited
-
Eliminate XLOG_HEAP2_VISIBLE from vacuum phase III
- add323da40a6 19 (unreleased) cited
-
Update various forward declarations to use typedef
- d4d1fc527bdb 19 (unreleased) cited
-
Pathify RHS unique-ification for semijoin planning
- 24225ad9aafc 19 (unreleased) cited
-
Revert "Don't lock partitions pruned by initial pruning"
- 1722d5eb05d8 18.0 cited
-
For inplace update, send nontransactional invalidations.
- 243e9b40f1b2 18.0 cited
-
Remove nearly-unused SizeOfIptrData macro.
- 8023b5827fba 10.0 cited
Attachments
- 0001-Removed-vci_set_merge_and_copy_trans_funcs.patch (text/x-patch) patch 0001
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?
--
Regards,
Timur Magomedov