On 12.01.24 12:16, Alvaro Herrera wrote:
In get_attstattarget() I think we should return 0 for dropped columns
without reading attstattarget, which is useless anyway, and if it did
happen to return non-null, it might cause us to do stuff, which would be
a waste.

I ended up deciding to get rid of get_attstattarget() altogether and just do
the fetching inline in examine_attribute().  Because the previous API and
what you are discussing here is over-designed, since the only caller doesn't
call it with dropped columns or system columns anyway.  This way these
issues are contained in the ANALYZE code, not in a very general place like
lsyscache.c.

Sounds good.

I have committed this first patch.

Maybe instead of having examine_attribute hand a -1 target to the
analyze functions, we could just put default_statistics_target there.
Analyze functions would never receive negative values, and we could
remove that from the analyze functions.  Maybe make
VacAttrStats->attstattarget unsigned while at it.  (This could be a
separate patch.)

But I now remembered why I didn't do this. The extended statistics code needs to know whether the statistics target was set or left as default, because it will then apply its own sequence of logic to determine a final value. (Maybe there is a way to untangle this further, but it's not as obvious as it seems.)

At which point I then realized that extended statistics have their own statistics target catalog field and command, and we really should change that to match the changes done to attstattarget. So here is another patch that does all that again for stxstattarget. It's meant to mirror the attstattarget changes exactly.

And of course the 0003 patch gets rid of it anyway.

I again didn't look at 0002 and 0003 very closely, but from 10,000 feet
it looks mostly reasonable -- but I think naming the struct
FormData_pg_attribute_extra is not a great idea, as it looks like there
would have to be a catalog named pg_attribute_extra -- and I don't think
I would make the "non-Data" pointer-to-struct typedef either.

I agree that this naming was problematic. After some introverted bikeshedding, I changed it to FormExtraData_pg_attribute. Obviously, other solutions are possible. I also removed the typedef as you suggested.
From 9199be09efcbca1b906b5c41e8524e68b1a6b64e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 16:44:55 +0100
Subject: [PATCH v4 1/3] Make stxstattarget nullable

To match attstattarget change (commit 4f622503d6d).

TODO: catversion
---
 doc/src/sgml/catalogs.sgml              | 28 ++++++++++++-------------
 doc/src/sgml/ref/alter_statistics.sgml  | 13 ++++++------
 src/backend/commands/statscmds.c        | 21 ++++++++++++++++---
 src/backend/parser/gram.y               |  4 ++--
 src/backend/statistics/extended_stats.c |  4 +++-
 src/bin/pg_dump/pg_dump.c               | 10 +++++----
 src/bin/psql/describe.c                 |  3 ++-
 src/include/catalog/pg_statistic_ext.h  |  6 +++---
 src/include/nodes/parsenodes.h          |  2 +-
 9 files changed, 56 insertions(+), 35 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index c15d861e823..34458df6d4c 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -7633,6 +7633,19 @@ <title><structname>pg_statistic_ext</structname> 
Columns</title>
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>stxkeys</structfield> <type>int2vector</type>
+       (references <link 
linkend="catalog-pg-attribute"><structname>pg_attribute</structname></link>.<structfield>attnum</structfield>)
+      </para>
+      <para>
+       An array of attribute numbers, indicating which table columns are
+       covered by this statistics object;
+       for example a value of <literal>1 3</literal> would
+       mean that the first and the third table columns are covered
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>stxstattarget</structfield> <type>int2</type>
@@ -7642,7 +7655,7 @@ <title><structname>pg_statistic_ext</structname> 
Columns</title>
        of statistics accumulated for this statistics object by
        <link linkend="sql-analyze"><command>ANALYZE</command></link>.
        A zero value indicates that no statistics should be collected.
-       A negative value says to use the maximum of the statistics targets of
+       A null value says to use the maximum of the statistics targets of
        the referenced columns, if set, or the system default statistics target.
        Positive values of <structfield>stxstattarget</structfield>
        determine the target number of <quote>most common values</quote>
@@ -7650,19 +7663,6 @@ <title><structname>pg_statistic_ext</structname> 
Columns</title>
       </para></entry>
      </row>
 
-     <row>
-      <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>stxkeys</structfield> <type>int2vector</type>
-       (references <link 
linkend="catalog-pg-attribute"><structname>pg_attribute</structname></link>.<structfield>attnum</structfield>)
-      </para>
-      <para>
-       An array of attribute numbers, indicating which table columns are
-       covered by this statistics object;
-       for example a value of <literal>1 3</literal> would
-       mean that the first and the third table columns are covered
-      </para></entry>
-     </row>
-
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>stxkind</structfield> <type>char[]</type>
diff --git a/doc/src/sgml/ref/alter_statistics.sgml 
b/doc/src/sgml/ref/alter_statistics.sgml
index 73cc9e830de..c16caa0b61b 100644
--- a/doc/src/sgml/ref/alter_statistics.sgml
+++ b/doc/src/sgml/ref/alter_statistics.sgml
@@ -26,7 +26,7 @@
 ALTER STATISTICS <replaceable class="parameter">name</replaceable> OWNER TO { 
<replaceable class="parameter">new_owner</replaceable> | CURRENT_ROLE | 
CURRENT_USER | SESSION_USER }
 ALTER STATISTICS <replaceable class="parameter">name</replaceable> RENAME TO 
<replaceable class="parameter">new_name</replaceable>
 ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA 
<replaceable class="parameter">new_schema</replaceable>
-ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET 
STATISTICS <replaceable class="parameter">new_target</replaceable>
+ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET 
STATISTICS { <replaceable class="parameter">integer</replaceable> | DEFAULT }
 </synopsis>
  </refsynopsisdiv>
 
@@ -96,15 +96,16 @@ <title>Parameters</title>
      </varlistentry>
 
      <varlistentry>
-      <term><replaceable class="parameter">new_target</replaceable></term>
+      <term><literal>SET STATISTICS { <replaceable 
class="parameter">integer</replaceable> | DEFAULT }</literal></term>
       <listitem>
        <para>
         The statistic-gathering target for this statistics object for 
subsequent
         <link linkend="sql-analyze"><command>ANALYZE</command></link> 
operations.
-        The target can be set in the range 0 to 10000; alternatively, set it
-        to -1 to revert to using the maximum of the statistics target of the
-        referenced columns, if set, or the system default statistics
-        target (<xref linkend="guc-default-statistics-target"/>).
+        The target can be set in the range 0 to 10000.  Set it to
+        <literal>DEFAULT</literal> to revert to using the system default
+        statistics target (<xref linkend="guc-default-statistics-target"/>).
+        (Setting to a value of -1 is an obsolete way spelling to get the same
+        outcome.)
         For more information on the use of statistics by the
         <productname>PostgreSQL</productname> query planner, refer to
         <xref linkend="planner-stats"/>.
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index b1a9c74bd63..b2e509afef0 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -498,9 +498,9 @@ CreateStatistics(CreateStatsStmt *stmt)
        values[Anum_pg_statistic_ext_stxrelid - 1] = ObjectIdGetDatum(relid);
        values[Anum_pg_statistic_ext_stxname - 1] = NameGetDatum(&stxname);
        values[Anum_pg_statistic_ext_stxnamespace - 1] = 
ObjectIdGetDatum(namespaceId);
-       values[Anum_pg_statistic_ext_stxstattarget - 1] = Int16GetDatum(-1);
        values[Anum_pg_statistic_ext_stxowner - 1] = ObjectIdGetDatum(stxowner);
        values[Anum_pg_statistic_ext_stxkeys - 1] = PointerGetDatum(stxkeys);
+       nulls[Anum_pg_statistic_ext_stxstattarget - 1] = true;
        values[Anum_pg_statistic_ext_stxkind - 1] = PointerGetDatum(stxkind);
 
        values[Anum_pg_statistic_ext_stxexprs - 1] = exprsDatum;
@@ -609,7 +609,19 @@ AlterStatistics(AlterStatsStmt *stmt)
        bool            repl_null[Natts_pg_statistic_ext];
        bool            repl_repl[Natts_pg_statistic_ext];
        ObjectAddress address;
-       int                     newtarget = stmt->stxstattarget;
+       int                     newtarget;
+
+       if (stmt->stxstattarget)
+       {
+               newtarget = intVal(stmt->stxstattarget);
+       }
+       else
+       {
+               /*
+                * -1 was used in previous versions to represent the default 
setting
+                */
+               newtarget = -1;
+       }
 
        /* Limit statistics target to a sane range */
        if (newtarget < -1)
@@ -676,7 +688,10 @@ AlterStatistics(AlterStatsStmt *stmt)
 
        /* replace the stxstattarget column */
        repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true;
-       repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = 
Int16GetDatum(newtarget);
+       if (newtarget != -1)
+               repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = 
Int16GetDatum(newtarget);
+       else
+               repl_null[Anum_pg_statistic_ext_stxstattarget - 1] = true;
 
        newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel),
                                                           repl_val, repl_null, 
repl_repl);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3460fea56ba..e0e30ce1716 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4597,7 +4597,7 @@ stats_param:      ColId
  *****************************************************************************/
 
 AlterStatsStmt:
-                       ALTER STATISTICS any_name SET STATISTICS SignedIconst
+                       ALTER STATISTICS any_name SET STATISTICS 
set_statistics_value
                                {
                                        AlterStatsStmt *n = 
makeNode(AlterStatsStmt);
 
@@ -4606,7 +4606,7 @@ AlterStatsStmt:
                                        n->stxstattarget = $6;
                                        $$ = (Node *) n;
                                }
-                       | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS 
SignedIconst
+                       | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS 
set_statistics_value
                                {
                                        AlterStatsStmt *n = 
makeNode(AlterStatsStmt);
 
diff --git a/src/backend/statistics/extended_stats.c 
b/src/backend/statistics/extended_stats.c
index c5461514d8f..33f1ad98d49 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -457,13 +457,15 @@ fetch_statentries_for_relation(Relation pg_statext, Oid 
relid)
                entry->statOid = staForm->oid;
                entry->schema = get_namespace_name(staForm->stxnamespace);
                entry->name = pstrdup(NameStr(staForm->stxname));
-               entry->stattarget = staForm->stxstattarget;
                for (i = 0; i < staForm->stxkeys.dim1; i++)
                {
                        entry->columns = bms_add_member(entry->columns,
                                                                                
        staForm->stxkeys.values[i]);
                }
 
+               datum = SysCacheGetAttr(STATEXTOID, htup, 
Anum_pg_statistic_ext_stxstattarget, &isnull);
+               entry->stattarget = isnull ? -1 : DatumGetInt16(datum);
+
                /* decode the stxkind char array into a list of chars */
                datum = SysCacheGetAttrNotNull(STATEXTOID, htup,
                                                                           
Anum_pg_statistic_ext_stxkind);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bc20a025ce4..b151eac6654 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7542,7 +7542,7 @@ getExtendedStatistics(Archive *fout)
 
        if (fout->remoteVersion < 130000)
                appendPQExpBufferStr(query, "SELECT tableoid, oid, stxname, "
-                                                        "stxnamespace, 
stxowner, stxrelid, (-1) AS stxstattarget "
+                                                        "stxnamespace, 
stxowner, stxrelid, NULL AS stxstattarget "
                                                         "FROM 
pg_catalog.pg_statistic_ext");
        else
                appendPQExpBufferStr(query, "SELECT tableoid, oid, stxname, "
@@ -7575,7 +7575,10 @@ getExtendedStatistics(Archive *fout)
                statsextinfo[i].rolname = getRoleName(PQgetvalue(res, i, 
i_stxowner));
                statsextinfo[i].stattable =
                        findTableByOid(atooid(PQgetvalue(res, i, i_stxrelid)));
-               statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, 
i_stattarget));
+               if (PQgetisnull(res, i, i_stattarget))
+                       statsextinfo[i].stattarget = -1;
+               else
+                       statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, 
i_stattarget));
 
                /* Decide whether we want to dump it */
                selectDumpableStatisticsObject(&(statsextinfo[i]), fout);
@@ -17010,8 +17013,7 @@ dumpStatisticsExt(Archive *fout, const StatsExtInfo 
*statsextinfo)
 
        /*
         * We only issue an ALTER STATISTICS statement if the stxstattarget 
entry
-        * for this statistics object is non-negative (i.e. it's not the default
-        * value).
+        * for this statistics object is not the default value.
         */
        if (statsextinfo->stattarget >= 0)
        {
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 37f95163201..7be1ffba1ee 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2792,7 +2792,8 @@ describeOneTableDetails(const char *schemaname,
                                                                          
PQgetvalue(result, i, 1));
 
                                        /* Show the stats target if it's not 
default */
-                                       if (strcmp(PQgetvalue(result, i, 8), 
"-1") != 0)
+                                       if (!PQgetisnull(result, i, 8) &&
+                                               strcmp(PQgetvalue(result, i, 
8), "-1") != 0)
                                                appendPQExpBuffer(&buf, "; 
STATISTICS %s",
                                                                                
  PQgetvalue(result, i, 8));
 
diff --git a/src/include/catalog/pg_statistic_ext.h 
b/src/include/catalog/pg_statistic_ext.h
index 104b7db1f95..1ef58b33e3e 100644
--- a/src/include/catalog/pg_statistic_ext.h
+++ b/src/include/catalog/pg_statistic_ext.h
@@ -43,15 +43,15 @@ CATALOG(pg_statistic_ext,3381,StatisticExtRelationId)
                                                                                
                                 * object's namespace */
 
        Oid                     stxowner BKI_LOOKUP(pg_authid); /* statistics 
object's owner */
-       int16           stxstattarget BKI_DEFAULT(-1);  /* statistics target */
 
        /*
-        * variable-length fields start here, but we allow direct access to
-        * stxkeys
+        * variable-length/nullable fields start here, but we allow direct 
access
+        * to stxkeys
         */
        int2vector      stxkeys BKI_FORCE_NOT_NULL; /* array of column keys */
 
 #ifdef CATALOG_VARLEN
+       int16           stxstattarget BKI_DEFAULT(_null_) BKI_FORCE_NULL;       
/* statistics target */
        char            stxkind[1] BKI_FORCE_NOT_NULL;  /* statistics kinds 
requested
                                                                                
                 * to build */
        pg_node_tree stxexprs;          /* A list of expression trees for stats
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index b3181f34aee..59ea2f0a9f0 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3267,7 +3267,7 @@ typedef struct AlterStatsStmt
 {
        NodeTag         type;
        List       *defnames;           /* qualified name (list of String) */
-       int                     stxstattarget;  /* statistics target */
+       Node       *stxstattarget;      /* statistics target */
        bool            missing_ok;             /* skip error if statistics 
object is missing */
 } AlterStatsStmt;
 

base-commit: 31acee4b66f9f88ad5c19c1276252688bdaa95c9
-- 
2.43.0

From 552e5cc5d5e78134dcfcea242037f6ef36b4c36a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 16:44:55 +0100
Subject: [PATCH v4 2/3] Generalize handling of nullable pg_attribute columns
 in DDL

DDL code uses tuple descriptors to pass around pg_attribute values
during table and index creation.  But tuple descriptors don't include
the variable-length/nullable columns of pg_attribute, so they have to
be handled separately.  Right now, the attoptions field is handled in
a one-off way with a separate argument passed to
InsertPgAttributeTuples().  The other affected fields of pg_attribute
are right now not needed at relation creation time.

The goal of this patch is to generalize this to allow handling
additional variable-length/nullable columns of pg_attribute in a
similar manner.  For that, create a new struct
FormExtraData_pg_attribute, which is to be passed around in parallel
to the tuple descriptor and optionally supplies the additional
columns.  Right now, this struct only contains one field for
attoptions, so no functionality is actually changed by this.

Discussion: 
https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184...@eisentraut.org
---
 src/backend/catalog/heap.c         | 12 +++++++++---
 src/backend/catalog/index.c        | 16 +++++++++++++++-
 src/include/catalog/heap.h         |  2 +-
 src/include/catalog/pg_attribute.h | 13 +++++++++++++
 src/tools/pgindent/typedefs.list   |  1 +
 5 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 45a71081d42..70e14d09263 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -697,7 +697,7 @@ void
 InsertPgAttributeTuples(Relation pg_attribute_rel,
                                                TupleDesc tupdesc,
                                                Oid new_rel_oid,
-                                               const Datum *attoptions,
+                                               const 
FormExtraData_pg_attribute tupdesc_extra[],
                                                CatalogIndexState indstate)
 {
        TupleTableSlot **slot;
@@ -719,6 +719,7 @@ InsertPgAttributeTuples(Relation pg_attribute_rel,
        while (natts < tupdesc->natts)
        {
                Form_pg_attribute attrs = TupleDescAttr(tupdesc, natts);
+               const FormExtraData_pg_attribute *attrs_extra = tupdesc_extra ? 
&tupdesc_extra[natts] : NULL;
 
                ExecClearTuple(slot[slotCount]);
 
@@ -750,10 +751,15 @@ InsertPgAttributeTuples(Relation pg_attribute_rel,
                slot[slotCount]->tts_values[Anum_pg_attribute_attislocal - 1] = 
BoolGetDatum(attrs->attislocal);
                slot[slotCount]->tts_values[Anum_pg_attribute_attinhcount - 1] 
= Int16GetDatum(attrs->attinhcount);
                slot[slotCount]->tts_values[Anum_pg_attribute_attcollation - 1] 
= ObjectIdGetDatum(attrs->attcollation);
-               if (attoptions && attoptions[natts] != (Datum) 0)
-                       
slot[slotCount]->tts_values[Anum_pg_attribute_attoptions - 1] = 
attoptions[natts];
+               if (attrs_extra)
+               {
+                       
slot[slotCount]->tts_values[Anum_pg_attribute_attoptions - 1] = 
attrs_extra->attoptions.value;
+                       
slot[slotCount]->tts_isnull[Anum_pg_attribute_attoptions - 1] = 
attrs_extra->attoptions.isnull;
+               }
                else
+               {
                        
slot[slotCount]->tts_isnull[Anum_pg_attribute_attoptions - 1] = true;
+               }
 
                /*
                 * The remaining fields are not set for new columns.
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index fbef3d5382d..f899d15876f 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -517,6 +517,20 @@ AppendAttributeTuples(Relation indexRelation, const Datum 
*attopts)
        Relation        pg_attribute;
        CatalogIndexState indstate;
        TupleDesc       indexTupDesc;
+       FormExtraData_pg_attribute *attrs_extra = NULL;
+
+       if (attopts)
+       {
+               attrs_extra = palloc0_array(FormExtraData_pg_attribute, 
indexRelation->rd_att->natts);
+
+               for (int i = 0; i < indexRelation->rd_att->natts; i++)
+               {
+                       if (attopts[i])
+                               attrs_extra[i].attoptions.value = attopts[i];
+                       else
+                               attrs_extra[i].attoptions.isnull = true;
+               }
+       }
 
        /*
         * open the attribute relation and its indexes
@@ -530,7 +544,7 @@ AppendAttributeTuples(Relation indexRelation, const Datum 
*attopts)
         */
        indexTupDesc = RelationGetDescr(indexRelation);
 
-       InsertPgAttributeTuples(pg_attribute, indexTupDesc, InvalidOid, 
attopts, indstate);
+       InsertPgAttributeTuples(pg_attribute, indexTupDesc, InvalidOid, 
attrs_extra, indstate);
 
        CatalogCloseIndexes(indstate);
 
diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h
index 1d7f8380d90..21e31f9c974 100644
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -98,7 +98,7 @@ extern List *heap_truncate_find_FKs(List *relationIds);
 extern void InsertPgAttributeTuples(Relation pg_attribute_rel,
                                                                        
TupleDesc tupdesc,
                                                                        Oid 
new_rel_oid,
-                                                                       const 
Datum *attoptions,
+                                                                       const 
FormExtraData_pg_attribute tupdesc_extra[],
                                                                        
CatalogIndexState indstate);
 
 extern void InsertPgClassTuple(Relation pg_class_desc,
diff --git a/src/include/catalog/pg_attribute.h 
b/src/include/catalog/pg_attribute.h
index e2aadb94141..27b0077e25b 100644
--- a/src/include/catalog/pg_attribute.h
+++ b/src/include/catalog/pg_attribute.h
@@ -208,6 +208,19 @@ CATALOG(pg_attribute,1249,AttributeRelationId) 
BKI_BOOTSTRAP BKI_ROWTYPE_OID(75,
  */
 typedef FormData_pg_attribute *Form_pg_attribute;
 
+/*
+ * FormExtraData_pg_attribute contains (some of) the fields that are not in
+ * FormData_pg_attribute because they are excluded by CATALOG_VARLEN.  It is
+ * meant to be used by DDL code so that the combination of
+ * FormData_pg_attribute (often via tuple descriptor) and
+ * FormExtraData_pg_attribute can be used to pass around all the information
+ * about an attribute.  Fields can be included here as needed.
+ */
+typedef struct FormExtraData_pg_attribute
+{
+       NullableDatum attoptions;
+} FormExtraData_pg_attribute;
+
 DECLARE_UNIQUE_INDEX(pg_attribute_relid_attnam_index, 2658, 
AttributeRelidNameIndexId, pg_attribute, btree(attrelid oid_ops, attname 
name_ops));
 DECLARE_UNIQUE_INDEX_PKEY(pg_attribute_relid_attnum_index, 2659, 
AttributeRelidNumIndexId, pg_attribute, btree(attrelid oid_ops, attnum 
int2_ops));
 
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index f582eb59e7d..d6d065375e9 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -844,6 +844,7 @@ FormData_pg_ts_parser
 FormData_pg_ts_template
 FormData_pg_type
 FormData_pg_user_mapping
+FormExtraData_pg_attribute
 Form_pg_aggregate
 Form_pg_am
 Form_pg_amop
-- 
2.43.0

From e12fa90ddc05f5ffc21f3240d3cb22beaa6969ac Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 16:44:56 +0100
Subject: [PATCH v4 3/3] Add attstattarget to FormExtraData_pg_attribute

This allows setting attstattarget when a relation is created.

We make use of this by having index_concurrently_create_copy() copy
over the attstattarget values when the new index is created, instead
of having index_concurrently_swap() fix it up later.

Discussion: 
https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184...@eisentraut.org
---
 src/backend/catalog/heap.c         |  5 +-
 src/backend/catalog/index.c        | 97 +++++++++---------------------
 src/backend/catalog/toasting.c     |  2 +-
 src/backend/commands/indexcmds.c   |  2 +-
 src/include/catalog/index.h        |  1 +
 src/include/catalog/pg_attribute.h |  1 +
 6 files changed, 36 insertions(+), 72 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 70e14d09263..182db28a54d 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -753,18 +753,21 @@ InsertPgAttributeTuples(Relation pg_attribute_rel,
                slot[slotCount]->tts_values[Anum_pg_attribute_attcollation - 1] 
= ObjectIdGetDatum(attrs->attcollation);
                if (attrs_extra)
                {
+                       
slot[slotCount]->tts_values[Anum_pg_attribute_attstattarget - 1] = 
attrs_extra->attstattarget.value;
+                       
slot[slotCount]->tts_isnull[Anum_pg_attribute_attstattarget - 1] = 
attrs_extra->attstattarget.isnull;
+
                        
slot[slotCount]->tts_values[Anum_pg_attribute_attoptions - 1] = 
attrs_extra->attoptions.value;
                        
slot[slotCount]->tts_isnull[Anum_pg_attribute_attoptions - 1] = 
attrs_extra->attoptions.isnull;
                }
                else
                {
+                       
slot[slotCount]->tts_isnull[Anum_pg_attribute_attstattarget - 1] = true;
                        
slot[slotCount]->tts_isnull[Anum_pg_attribute_attoptions - 1] = true;
                }
 
                /*
                 * The remaining fields are not set for new columns.
                 */
-               slot[slotCount]->tts_isnull[Anum_pg_attribute_attstattarget - 
1] = true;
                slot[slotCount]->tts_isnull[Anum_pg_attribute_attacl - 1] = 
true;
                slot[slotCount]->tts_isnull[Anum_pg_attribute_attfdwoptions - 
1] = true;
                slot[slotCount]->tts_isnull[Anum_pg_attribute_attmissingval - 
1] = true;
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f899d15876f..91ce3a21b29 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -112,7 +112,7 @@ static TupleDesc ConstructTupleDescriptor(Relation 
heapRelation,
                                                                                
  const Oid *opclassIds);
 static void InitializeAttributeOids(Relation indexRelation,
                                                                        int 
numatts, Oid indexoid);
-static void AppendAttributeTuples(Relation indexRelation, const Datum 
*attopts);
+static void AppendAttributeTuples(Relation indexRelation, const Datum 
*attopts, const NullableDatum *stattargets);
 static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
                                                                Oid 
parentIndexId,
                                                                const IndexInfo 
*indexInfo,
@@ -512,7 +512,7 @@ InitializeAttributeOids(Relation indexRelation,
  * ----------------------------------------------------------------
  */
 static void
-AppendAttributeTuples(Relation indexRelation, const Datum *attopts)
+AppendAttributeTuples(Relation indexRelation, const Datum *attopts, const 
NullableDatum *stattargets)
 {
        Relation        pg_attribute;
        CatalogIndexState indstate;
@@ -529,6 +529,11 @@ AppendAttributeTuples(Relation indexRelation, const Datum 
*attopts)
                                attrs_extra[i].attoptions.value = attopts[i];
                        else
                                attrs_extra[i].attoptions.isnull = true;
+
+                       if (stattargets)
+                               attrs_extra[i].attstattarget = stattargets[i];
+                       else
+                               attrs_extra[i].attstattarget.isnull = true;
                }
        }
 
@@ -735,6 +740,7 @@ index_create(Relation heapRelation,
                         const Oid *opclassIds,
                         const Datum *opclassOptions,
                         const int16 *coloptions,
+                        const NullableDatum *stattargets,
                         Datum reloptions,
                         bits16 flags,
                         bits16 constr_flags,
@@ -1029,7 +1035,7 @@ index_create(Relation heapRelation,
        /*
         * append ATTRIBUTE tuples for the index
         */
-       AppendAttributeTuples(indexRelation, opclassOptions);
+       AppendAttributeTuples(indexRelation, opclassOptions, stattargets);
 
        /* ----------------
         *        update pg_index
@@ -1308,6 +1314,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid 
oldIndexId,
        Datum      *opclassOptions;
        oidvector  *indclass;
        int2vector *indcoloptions;
+       NullableDatum *stattargets;
        bool            isnull;
        List       *indexColNames = NIL;
        List       *indexExprs = NIL;
@@ -1412,6 +1419,23 @@ index_concurrently_create_copy(Relation heapRelation, 
Oid oldIndexId,
        for (int i = 0; i < newInfo->ii_NumIndexAttrs; i++)
                opclassOptions[i] = get_attoptions(oldIndexId, i + 1);
 
+       /* Extra statistic targets for each attribute */
+       stattargets = palloc0_array(NullableDatum, newInfo->ii_NumIndexAttrs);
+       for (int i = 0; i < newInfo->ii_NumIndexAttrs; i++)
+       {
+               HeapTuple   tp;
+               Datum       dat;
+
+               tp = SearchSysCache2(ATTNUM, ObjectIdGetDatum(oldIndexId), 
Int16GetDatum(i + 1));
+               if (!HeapTupleIsValid(tp))
+                       elog(ERROR, "cache lookup failed for attribute %d of 
relation %u",
+                                i + 1, oldIndexId);
+               dat = SysCacheGetAttr(ATTNUM, tp, 
Anum_pg_attribute_attstattarget, &isnull);
+               ReleaseSysCache(tp);
+               stattargets[i].value = dat;
+               stattargets[i].isnull = isnull;
+       }
+
        /*
         * Now create the new index.
         *
@@ -1433,6 +1457,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid 
oldIndexId,
                                                          indclass->values,
                                                          opclassOptions,
                                                          indcoloptions->values,
+                                                         stattargets,
                                                          reloptionsDatum,
                                                          
INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT,
                                                          0,
@@ -1775,72 +1800,6 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, 
const char *oldName)
        /* Copy data of pg_statistic from the old index to the new one */
        CopyStatistics(oldIndexId, newIndexId);
 
-       /* Copy pg_attribute.attstattarget for each index attribute */
-       {
-               HeapTuple       attrTuple;
-               Relation        pg_attribute;
-               SysScanDesc scan;
-               ScanKeyData key[1];
-
-               pg_attribute = table_open(AttributeRelationId, 
RowExclusiveLock);
-               ScanKeyInit(&key[0],
-                                       Anum_pg_attribute_attrelid,
-                                       BTEqualStrategyNumber, F_OIDEQ,
-                                       ObjectIdGetDatum(newIndexId));
-               scan = systable_beginscan(pg_attribute, 
AttributeRelidNumIndexId,
-                                                                 true, NULL, 
1, key);
-
-               while (HeapTupleIsValid((attrTuple = systable_getnext(scan))))
-               {
-                       Form_pg_attribute att = (Form_pg_attribute) 
GETSTRUCT(attrTuple);
-                       HeapTuple       tp;
-                       Datum           dat;
-                       bool            isnull;
-                       Datum           repl_val[Natts_pg_attribute];
-                       bool            repl_null[Natts_pg_attribute];
-                       bool            repl_repl[Natts_pg_attribute];
-                       HeapTuple       newTuple;
-
-                       /* Ignore dropped columns */
-                       if (att->attisdropped)
-                               continue;
-
-                       /*
-                        * Get attstattarget from the old index and refresh the 
new value.
-                        */
-                       tp = SearchSysCache2(ATTNUM, 
ObjectIdGetDatum(oldIndexId), Int16GetDatum(att->attnum));
-                       if (!HeapTupleIsValid(tp))
-                               elog(ERROR, "cache lookup failed for attribute 
%d of relation %u",
-                                        att->attnum, oldIndexId);
-                       dat = SysCacheGetAttr(ATTNUM, tp, 
Anum_pg_attribute_attstattarget, &isnull);
-                       ReleaseSysCache(tp);
-
-                       /*
-                        * No need for a refresh if old index value is null.  
(All new
-                        * index values are null at this point.)
-                        */
-                       if (isnull)
-                               continue;
-
-                       memset(repl_val, 0, sizeof(repl_val));
-                       memset(repl_null, false, sizeof(repl_null));
-                       memset(repl_repl, false, sizeof(repl_repl));
-
-                       repl_repl[Anum_pg_attribute_attstattarget - 1] = true;
-                       repl_val[Anum_pg_attribute_attstattarget - 1] = dat;
-
-                       newTuple = heap_modify_tuple(attrTuple,
-                                                                               
 RelationGetDescr(pg_attribute),
-                                                                               
 repl_val, repl_null, repl_repl);
-                       CatalogTupleUpdate(pg_attribute, &newTuple->t_self, 
newTuple);
-
-                       heap_freetuple(newTuple);
-               }
-
-               systable_endscan(scan);
-               table_close(pg_attribute, RowExclusiveLock);
-       }
-
        /* Close relations */
        table_close(pg_class, RowExclusiveLock);
        table_close(pg_index, RowExclusiveLock);
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 05d945b34b7..bbdf74ebed3 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -326,7 +326,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid 
toastIndexOid,
                                 list_make2("chunk_id", "chunk_seq"),
                                 BTREE_AM_OID,
                                 rel->rd_rel->reltablespace,
-                                collationIds, opclassIds, NULL, coloptions, 
(Datum) 0,
+                                collationIds, opclassIds, NULL, coloptions, 
NULL, (Datum) 0,
                                 INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
 
        table_close(toast_rel, NoLock);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 340248a3f29..c2416c11d61 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1193,7 +1193,7 @@ DefineIndex(Oid tableId,
                                         stmt->oldNumber, indexInfo, 
indexColNames,
                                         accessMethodId, tablespaceId,
                                         collationIds, opclassIds, 
opclassOptions,
-                                        coloptions, reloptions,
+                                        coloptions, NULL, reloptions,
                                         flags, constr_flags,
                                         allowSystemTableMods, !check_rights,
                                         &createdConstraintId);
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 99dab5940bc..7d434f8e653 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -80,6 +80,7 @@ extern Oid    index_create(Relation heapRelation,
                                                 const Oid *opclassIds,
                                                 const Datum *opclassOptions,
                                                 const int16 *coloptions,
+                                                const NullableDatum 
*stattargets,
                                                 Datum reloptions,
                                                 bits16 flags,
                                                 bits16 constr_flags,
diff --git a/src/include/catalog/pg_attribute.h 
b/src/include/catalog/pg_attribute.h
index 27b0077e25b..b28725f4dbe 100644
--- a/src/include/catalog/pg_attribute.h
+++ b/src/include/catalog/pg_attribute.h
@@ -218,6 +218,7 @@ typedef FormData_pg_attribute *Form_pg_attribute;
  */
 typedef struct FormExtraData_pg_attribute
 {
+       NullableDatum attstattarget;
        NullableDatum attoptions;
 } FormExtraData_pg_attribute;
 
-- 
2.43.0

Reply via email to