On 6/10/26 08:21, Michael Paquier wrote:
Planting an assertion at this location feels OK here. (I need to remember to look at that again once v20 opens for business, noting that down now..) -- Michael
Hi Michael, Thanks for the previous review. I have attached two small patches:- v3 removes the obsolete tupDesc assignment in extended statistics and retains the assertion suggested in your review. It has been rebased on the current master. - v1 removes a stray semicolon after import_expressions(), which I noticed while working in the same area.
The second change is independent, but small enough that I have included it here rather than starting a separate thread.
Could you please take a look and commit them if they look good? Best regards, Denis Rodionov, Tantor Labs LLC, https://tantorlabs.com/
From df2eacc607893c02f33a9da8dfd2a371aadb1645 Mon Sep 17 00:00:00 2001 From: Denis Rodionov <[email protected]> Date: Thu, 4 Jun 2026 20:23:07 +0300 Subject: [PATCH v3] Remove obsolete tupDesc assignment in extended statistics lookup_var_attr_stats() assigned tupDesc to VacAttrStats entries built for expressions. That appears to be obsolete: make_build_data() uses tupDesc only for regular columns, while expression values are evaluated separately. Remove the assignment and add an assertion documenting that expression stats do not need a tuple descriptor. --- src/backend/statistics/extended_stats.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index 332e7423bd8..a572aebfee9 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -786,12 +786,10 @@ lookup_var_attr_stats(Bitmapset *attrs, List *exprs, } /* - * XXX We need tuple descriptor later, and we just grab it from - * stats[0]->tupDesc (see e.g. statext_mcv_build). But as coded - * examine_attribute does not set that, so just grab it from the first - * vacatts element. + * Expression stats are not tied to a heap attribute, so they do not + * need a tuple descriptor. */ - stats[i]->tupDesc = vacatts[0]->tupDesc; + Assert(stats[i]->tupDesc == NULL); i++; } -- 2.34.1
From caff19ccbb087ea691697f6b1eea515140b78a13 Mon Sep 17 00:00:00 2001 From: Denis Rodionov <[email protected]> Date: Wed, 29 Jul 2026 16:57:51 +0300 Subject: [PATCH v1] Remove stray semicolon after import_expressions() --- src/backend/statistics/extended_stats_funcs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/statistics/extended_stats_funcs.c b/src/backend/statistics/extended_stats_funcs.c index a3e56933b91..789f0f31ecd 100644 --- a/src/backend/statistics/extended_stats_funcs.c +++ b/src/backend/statistics/extended_stats_funcs.c @@ -1682,7 +1682,7 @@ exprs_error: if (astate != NULL) pfree(astate); return (Datum) 0; -}; +} /* * Remove an existing pg_statistic_ext_data row for a given pg_statistic_ext -- 2.34.1
