Take pg_attribute out of VacAttrStats
authorPeter Eisentraut <peter@eisentraut.org>
Mon, 3 Jul 2023 05:09:22 +0000 (07:09 +0200)
committerPeter Eisentraut <peter@eisentraut.org>
Mon, 3 Jul 2023 05:18:57 +0000 (07:18 +0200)
The VacAttrStats structure contained the whole Form_pg_attribute for a
column, but it actually only needs attstattarget from there.  So
remove the Form_pg_attribute field and make a separate field for
attstattarget.  This simplifies some code for extended statistics that
doesn't deal with a column but an expression, which had to fake up
pg_attribute rows to satisfy internal APIs.  Also, we can remove some
comments that essentially said "don't look at pg_attribute directly".

Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/d6069765-5971-04d3-c10d-e4f7b2e9c459%40eisentraut.org

src/backend/commands/analyze.c
src/backend/statistics/extended_stats.c
src/backend/tsearch/ts_typanalyze.c
src/backend/utils/adt/array_typanalyze.c
src/backend/utils/adt/rangetypes_typanalyze.c
src/include/commands/vacuum.h

index fc9a371f9be790967682724e112fff9577abf7ad..9c8413eef23590a858f3c7bc2a5eef9b9bb83c83 100644 (file)
@@ -572,7 +572,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
                         * If the appropriate flavor of the n_distinct option is
                         * specified, override with the corresponding value.
                         */
-                       aopt = get_attribute_options(onerel->rd_id, stats->attr->attnum);
+                       aopt = get_attribute_options(onerel->rd_id, stats->tupattnum);
                        if (aopt != NULL)
                        {
                                float8          n_distinct;
@@ -927,7 +927,7 @@ compute_index_stats(Relation onerel, double totalrows,
                                for (i = 0; i < attr_cnt; i++)
                                {
                                        VacAttrStats *stats = thisdata->vacattrstats[i];
-                                       int                     attnum = stats->attr->attnum;
+                                       int                     attnum = stats->tupattnum;
 
                                        if (isnull[attnum - 1])
                                        {
@@ -1014,12 +1014,10 @@ examine_attribute(Relation onerel, int attnum, Node *index_expr)
                return NULL;
 
        /*
-        * Create the VacAttrStats struct.  Note that we only have a copy of the
-        * fixed fields of the pg_attribute tuple.
+        * Create the VacAttrStats struct.
         */
        stats = (VacAttrStats *) palloc0(sizeof(VacAttrStats));
-       stats->attr = (Form_pg_attribute) palloc(ATTRIBUTE_FIXED_PART_SIZE);
-       memcpy(stats->attr, attr, ATTRIBUTE_FIXED_PART_SIZE);
+       stats->attstattarget = attr->attstattarget;
 
        /*
         * When analyzing an expression index, believe the expression tree's type
@@ -1086,7 +1084,6 @@ examine_attribute(Relation onerel, int attnum, Node *index_expr)
        if (!ok || stats->compute_stats == NULL || stats->minrows <= 0)
        {
                heap_freetuple(typtuple);
-               pfree(stats->attr);
                pfree(stats);
                return NULL;
        }
@@ -1659,7 +1656,7 @@ update_attstats(Oid relid, bool inh, int natts, VacAttrStats **vacattrstats)
                }
 
                values[Anum_pg_statistic_starelid - 1] = ObjectIdGetDatum(relid);
-               values[Anum_pg_statistic_staattnum - 1] = Int16GetDatum(stats->attr->attnum);
+               values[Anum_pg_statistic_staattnum - 1] = Int16GetDatum(stats->tupattnum);
                values[Anum_pg_statistic_stainherit - 1] = BoolGetDatum(inh);
                values[Anum_pg_statistic_stanullfrac - 1] = Float4GetDatum(stats->stanullfrac);
                values[Anum_pg_statistic_stawidth - 1] = Int32GetDatum(stats->stawidth);
@@ -1725,7 +1722,7 @@ update_attstats(Oid relid, bool inh, int natts, VacAttrStats **vacattrstats)
                /* Is there already a pg_statistic tuple for this attribute? */
                oldtup = SearchSysCache3(STATRELATTINH,
                                                                 ObjectIdGetDatum(relid),
-                                                                Int16GetDatum(stats->attr->attnum),
+                                                                Int16GetDatum(stats->tupattnum),
                                                                 BoolGetDatum(inh));
 
                /* Open index information when we know we need it */
@@ -1860,15 +1857,13 @@ static int      analyze_mcv_list(int *mcv_counts,
 bool
 std_typanalyze(VacAttrStats *stats)
 {
-       Form_pg_attribute attr = stats->attr;
        Oid                     ltopr;
        Oid                     eqopr;
        StdAnalyzeData *mystats;
 
        /* If the attstattarget column is negative, use the default value */
-       /* NB: it is okay to scribble on stats->attr since it's a copy */
-       if (attr->attstattarget < 0)
-               attr->attstattarget = default_statistics_target;
+       if (stats->attstattarget < 0)
+               stats->attstattarget = default_statistics_target;
 
        /* Look for default "<" and "=" operators for column's type */
        get_sort_group_operators(stats->attrtypid,
@@ -1909,21 +1904,21 @@ std_typanalyze(VacAttrStats *stats)
                 * know it at this point.
                 *--------------------
                 */
-               stats->minrows = 300 * attr->attstattarget;
+               stats->minrows = 300 * stats->attstattarget;
        }
        else if (OidIsValid(eqopr))
        {
                /* We can still recognize distinct values */
                stats->compute_stats = compute_distinct_stats;
                /* Might as well use the same minrows as above */
-               stats->minrows = 300 * attr->attstattarget;
+               stats->minrows = 300 * stats->attstattarget;
        }
        else
        {
                /* Can't do much but the trivial stuff */
                stats->compute_stats = compute_trivial_stats;
                /* Might as well use the same minrows as above */
-               stats->minrows = 300 * attr->attstattarget;
+               stats->minrows = 300 * stats->attstattarget;
        }
 
        return true;
@@ -2051,7 +2046,7 @@ compute_distinct_stats(VacAttrStatsP stats,
        TrackItem  *track;
        int                     track_cnt,
                                track_max;
-       int                     num_mcv = stats->attr->attstattarget;
+       int                     num_mcv = stats->attstattarget;
        StdAnalyzeData *mystats = (StdAnalyzeData *) stats->extra_data;
 
        /*
@@ -2392,8 +2387,8 @@ compute_scalar_stats(VacAttrStatsP stats,
        int                *tupnoLink;
        ScalarMCVItem *track;
        int                     track_cnt = 0;
-       int                     num_mcv = stats->attr->attstattarget;
-       int                     num_bins = stats->attr->attstattarget;
+       int                     num_mcv = stats->attstattarget;
+       int                     num_bins = stats->attstattarget;
        StdAnalyzeData *mystats = (StdAnalyzeData *) stats->extra_data;
 
        values = (ScalarItem *) palloc(samplerows * sizeof(ScalarItem));
index 513ddf540abfc875d9eac138c2f237043bee0519..9f67a577244a365eaa2b9a0e478a0f123443c541 100644 (file)
@@ -366,8 +366,8 @@ statext_compute_stattarget(int stattarget, int nattrs, VacAttrStats **stats)
        for (i = 0; i < nattrs; i++)
        {
                /* keep the maximum statistics target */
-               if (stats[i]->attr->attstattarget > stattarget)
-                       stattarget = stats[i]->attr->attstattarget;
+               if (stats[i]->attstattarget > stattarget)
+                       stattarget = stats[i]->attstattarget;
        }
 
        /*
@@ -534,14 +534,10 @@ examine_attribute(Node *expr)
        bool            ok;
 
        /*
-        * Create the VacAttrStats struct.  Note that we only have a copy of the
-        * fixed fields of the pg_attribute tuple.
+        * Create the VacAttrStats struct.
         */
        stats = (VacAttrStats *) palloc0(sizeof(VacAttrStats));
-
-       /* fake the attribute */
-       stats->attr = (Form_pg_attribute) palloc0(ATTRIBUTE_FIXED_PART_SIZE);
-       stats->attr->attstattarget = -1;
+       stats->attstattarget = -1;
 
        /*
         * When analyzing an expression, believe the expression tree's type not
@@ -595,7 +591,6 @@ examine_attribute(Node *expr)
        if (!ok || stats->compute_stats == NULL || stats->minrows <= 0)
        {
                heap_freetuple(typtuple);
-               pfree(stats->attr);
                pfree(stats);
                return NULL;
        }
@@ -624,6 +619,13 @@ examine_expression(Node *expr, int stattarget)
         */
        stats = (VacAttrStats *) palloc0(sizeof(VacAttrStats));
 
+       /*
+        * We can't have statistics target specified for the expression, so we
+        * could use either the default_statistics_target, or the target computed
+        * for the extended statistics. The second option seems more reasonable.
+        */
+       stats->attstattarget = stattarget;
+
        /*
         * When analyzing an expression, believe the expression tree's type.
         */
@@ -638,25 +640,6 @@ examine_expression(Node *expr, int stattarget)
         */
        stats->attrcollid = exprCollation(expr);
 
-       /*
-        * We don't have any pg_attribute for expressions, so let's fake something
-        * reasonable into attstattarget, which is the only thing std_typanalyze
-        * needs.
-        */
-       stats->attr = (Form_pg_attribute) palloc(ATTRIBUTE_FIXED_PART_SIZE);
-
-       /*
-        * We can't have statistics target specified for the expression, so we
-        * could use either the default_statistics_target, or the target computed
-        * for the extended statistics. The second option seems more reasonable.
-        */
-       stats->attr->attstattarget = stattarget;
-
-       /* initialize some basic fields */
-       stats->attr->attrelid = InvalidOid;
-       stats->attr->attnum = InvalidAttrNumber;
-       stats->attr->atttypid = stats->attrtypid;
-
        typtuple = SearchSysCacheCopy1(TYPEOID,
                                                                   ObjectIdGetDatum(stats->attrtypid));
        if (!HeapTupleIsValid(typtuple))
@@ -747,12 +730,6 @@ lookup_var_attr_stats(Relation rel, Bitmapset *attrs, List *exprs,
                        return NULL;
                }
 
-               /*
-                * Sanity check that the column is not dropped - stats should have
-                * been removed in this case.
-                */
-               Assert(!stats[i]->attr->attisdropped);
-
                i++;
        }
 
@@ -2237,8 +2214,7 @@ compute_expr_stats(Relation onerel, double totalrows,
                if (tcnt > 0)
                {
                        AttributeOpts *aopt =
-                               get_attribute_options(stats->attr->attrelid,
-                                                                         stats->attr->attnum);
+                               get_attribute_options(onerel->rd_id, stats->tupattnum);
 
                        stats->exprvals = exprvals;
                        stats->exprnulls = exprnulls;
index 75ecd72efe1e89381bf3b179d9a1da656c072571..659432273328712e11098afc6668d8b03bbf3e76 100644 (file)
@@ -58,16 +58,14 @@ Datum
 ts_typanalyze(PG_FUNCTION_ARGS)
 {
        VacAttrStats *stats = (VacAttrStats *) PG_GETARG_POINTER(0);
-       Form_pg_attribute attr = stats->attr;
 
        /* If the attstattarget column is negative, use the default value */
-       /* NB: it is okay to scribble on stats->attr since it's a copy */
-       if (attr->attstattarget < 0)
-               attr->attstattarget = default_statistics_target;
+       if (stats->attstattarget < 0)
+               stats->attstattarget = default_statistics_target;
 
        stats->compute_stats = compute_tsvector_stats;
        /* see comment about the choice of minrows in commands/analyze.c */
-       stats->minrows = 300 * attr->attstattarget;
+       stats->minrows = 300 * stats->attstattarget;
 
        PG_RETURN_BOOL(true);
 }
@@ -169,7 +167,7 @@ compute_tsvector_stats(VacAttrStats *stats,
         * the number of individual lexeme values tracked in pg_statistic ought to
         * be more than the number of values for a simple scalar column.
         */
-       num_mcelem = stats->attr->attstattarget * 10;
+       num_mcelem = stats->attstattarget * 10;
 
        /*
         * We set bucket width equal to (num_mcelem + 10) / 0.007 as per the
index 52e160d6bbb200ec8ed7cab830eb0fd0425cdb13..04b3764b686cf7fb617867f2059bb5722f398bdb 100644 (file)
@@ -263,7 +263,7 @@ compute_array_stats(VacAttrStats *stats, AnalyzeAttrFetchFunc fetchfunc,
         * the number of individual elements tracked in pg_statistic ought to be
         * more than the number of values for a simple scalar column.
         */
-       num_mcelem = stats->attr->attstattarget * 10;
+       num_mcelem = stats->attstattarget * 10;
 
        /*
         * We set bucket width equal to num_mcelem / 0.007 as per the comment
@@ -575,7 +575,7 @@ compute_array_stats(VacAttrStats *stats, AnalyzeAttrFetchFunc fetchfunc,
                count_items_count = hash_get_num_entries(count_tab);
                if (count_items_count > 0)
                {
-                       int                     num_hist = stats->attr->attstattarget;
+                       int                     num_hist = stats->attstattarget;
                        DECountItem **sorted_count_items;
                        int                     j;
                        int                     delta;
index 86810a1a6e6158591688638b890ac064658239f1..d13d8d2c53018bd1b1c7e3abf764c5e0822854c2 100644 (file)
@@ -47,18 +47,17 @@ range_typanalyze(PG_FUNCTION_ARGS)
 {
        VacAttrStats *stats = (VacAttrStats *) PG_GETARG_POINTER(0);
        TypeCacheEntry *typcache;
-       Form_pg_attribute attr = stats->attr;
 
        /* Get information about range type; note column might be a domain */
        typcache = range_get_typcache(fcinfo, getBaseType(stats->attrtypid));
 
-       if (attr->attstattarget < 0)
-               attr->attstattarget = default_statistics_target;
+       if (stats->attstattarget < 0)
+               stats->attstattarget = default_statistics_target;
 
        stats->compute_stats = compute_range_stats;
        stats->extra_data = typcache;
        /* same as in std_typanalyze */
-       stats->minrows = 300 * attr->attstattarget;
+       stats->minrows = 300 * stats->attstattarget;
 
        PG_RETURN_BOOL(true);
 }
@@ -74,18 +73,17 @@ multirange_typanalyze(PG_FUNCTION_ARGS)
 {
        VacAttrStats *stats = (VacAttrStats *) PG_GETARG_POINTER(0);
        TypeCacheEntry *typcache;
-       Form_pg_attribute attr = stats->attr;
 
        /* Get information about multirange type; note column might be a domain */
        typcache = multirange_get_typcache(fcinfo, getBaseType(stats->attrtypid));
 
-       if (attr->attstattarget < 0)
-               attr->attstattarget = default_statistics_target;
+       if (stats->attstattarget < 0)
+               stats->attstattarget = default_statistics_target;
 
        stats->compute_stats = compute_range_stats;
        stats->extra_data = typcache;
        /* same as in std_typanalyze */
-       stats->minrows = 300 * attr->attstattarget;
+       stats->minrows = 300 * stats->attstattarget;
 
        PG_RETURN_BOOL(true);
 }
@@ -136,7 +134,7 @@ compute_range_stats(VacAttrStats *stats, AnalyzeAttrFetchFunc fetchfunc,
        int                     empty_cnt = 0;
        int                     range_no;
        int                     slot_idx;
-       int                     num_bins = stats->attr->attstattarget;
+       int                     num_bins = stats->attstattarget;
        int                     num_hist;
        float8     *lengths;
        RangeBound *lowers,
index bda7792770a68ab8ebbaa7f3ac860e44324a869a..b027fb2c67f65ba49d5046666e95f35e466db3eb 100644 (file)
@@ -116,16 +116,12 @@ typedef struct VacAttrStats
 {
        /*
         * These fields are set up by the main ANALYZE code before invoking the
-        * type-specific typanalyze function.
-        *
-        * Note: do not assume that the data being analyzed has the same datatype
-        * shown in attr, ie do not trust attr->atttypid, attlen, etc.  This is
-        * because some index opclasses store a different type than the underlying
-        * column/expression.  Instead use attrtypid, attrtypmod, and attrtype for
+        * type-specific typanalyze function.  They don't necessarily match what
+        * is in pg_attribute, because some index opclasses store a different type
+        * than the underlying column/expression.  Therefore, use these fields for
         * information about the datatype being fed to the typanalyze function.
-        * Likewise, use attrcollid not attr->attcollation.
         */
-       Form_pg_attribute attr;         /* copy of pg_attribute row for column */
+       int                     attstattarget;
        Oid                     attrtypid;              /* type of data being analyzed */
        int32           attrtypmod;             /* typmod of data being analyzed */
        Form_pg_type attrtype;          /* copy of pg_type row for attrtypid */