v23-0002-Allow-negative-aggtransspace-to-indicate-unbound.patch

From 48b807a93c29c534c0151b950563b28021acd8c1 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Fri, 12 Sep 2025 13:11:47 +0900
Subject: [PATCH v23 2/2] Allow negative aggtransspace to indicate unbounded
 state size

This patch reuses the existing aggtransspace in pg_aggregate to
signal that an aggregate's transition state can grow unboundedly.  If
aggtransspace is set to a negative value, it now indicates that the
transition state may consume unpredictable or large amounts of memory,
such as in aggregates like array_agg or string_agg that accumulate
input rows.

This information can be used by the planner to avoid applying
memory-sensitive optimizations (e.g., eager aggregation) when there is
a risk of excessive memory usage during partial aggregation.

Bump catalog version.
---
 doc/src/sgml/catalogs.sgml               |  5 ++++-
 doc/src/sgml/ref/create_aggregate.sgml   | 11 ++++++++---
 src/backend/optimizer/plan/initsplan.c   | 23 +++++++----------------
 src/include/catalog/pg_aggregate.dat     | 10 ++++++----
 src/test/regress/expected/opr_sanity.out |  2 +-
 src/test/regress/sql/opr_sanity.sql      |  2 +-
 6 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index e9095bedf21..3acc2222a87 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -596,7 +596,10 @@
       </para>
       <para>
        Approximate average size (in bytes) of the transition state
-       data, or zero to use a default estimate
+       data. A positive value provides an estimate; zero means to
+       use a default estimate. A negative value indicates the state
+       data can grow unboundedly in size, such as when the aggregate
+       accumulates input rows (e.g., array_agg, string_agg).
       </para></entry>
      </row>
 
diff --git a/doc/src/sgml/ref/create_aggregate.sgml b/doc/src/sgml/ref/create_aggregate.sgml
index 222e0aa5c9d..0472ac2e874 100644
--- a/doc/src/sgml/ref/create_aggregate.sgml
+++ b/doc/src/sgml/ref/create_aggregate.sgml
@@ -384,9 +384,13 @@ SELECT col FROM tab ORDER BY col USING sortop LIMIT 1;
      <para>
       The approximate average size (in bytes) of the aggregate's state value.
       If this parameter is omitted or is zero, a default estimate is used
-      based on the <replaceable>state_data_type</replaceable>.
+      based on the <replaceable>state_data_type</replaceable>. If set to a
+      negative value, it indicates the state data can grow unboundedly in
+      size, such as when the aggregate accumulates input rows (e.g.,
+      array_agg, string_agg).
       The planner uses this value to estimate the memory required for a
-      grouped aggregate query.
+      grouped aggregate query and to avoid optimizations that may cause
+      excessive memory usage.
      </para>
     </listitem>
    </varlistentry>
@@ -568,7 +572,8 @@ SELECT col FROM tab ORDER BY col USING sortop LIMIT 1;
      <para>
       The approximate average size (in bytes) of the aggregate's state
       value, when using moving-aggregate mode.  This works the same as
-      <replaceable>state_data_size</replaceable>.
+      <replaceable>state_data_size</replaceable>, except that negative
+      values are not used to indicate unbounded state size.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 1af43bb60d2..b8d1c7e88a3 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -719,19 +719,14 @@ setup_eager_aggregation(PlannerInfo *root)
 
 /*
  * is_partial_agg_memory_risky
- *	  Checks if any aggregate poses a risk of excessive memory usage during
+ *	  Check if any aggregate poses a risk of excessive memory usage during
  *	  partial aggregation.
  *
- * We check if any aggregate uses INTERNAL transition type.  Although INTERNAL
- * is marked as pass-by-value, it usually points to a large internal data
- * structure (like those used by string_agg or array_agg).  These transition
- * states can grow large and their size is hard to estimate.  Applying eager
- * aggregation in such cases risks high memory usage since partial aggregation
- * results might be stored in join hash tables or materialized nodes.
- *
- * We explicitly exclude aggregates with AVG_ACCUM transition function from
- * this check, based on the assumption that avg() and sum() are safe in this
- * context.
+ * We check if any aggregate has a negative aggtransspace value, which
+ * indicates that its transition state data can grow unboundedly in size.
+ * Applying eager aggregation in such cases risks high memory usage since
+ * partial aggregation results might be stored in join hash tables or
+ * materialized nodes.
  */
 static bool
 is_partial_agg_memory_risky(PlannerInfo *root)
@@ -742,11 +737,7 @@ is_partial_agg_memory_risky(PlannerInfo *root)
 	{
 		AggTransInfo *transinfo = lfirst_node(AggTransInfo, lc);
 
-		if (transinfo->transfn_oid == F_NUMERIC_AVG_ACCUM ||
-			transinfo->transfn_oid == F_INT8_AVG_ACCUM)
-			continue;
-
-		if (transinfo->aggtranstype == INTERNALOID)
+		if (transinfo->aggtransspace < 0)
 			return true;
 	}
 
diff --git a/src/include/catalog/pg_aggregate.dat b/src/include/catalog/pg_aggregate.dat
index d6aa1f6ec47..870769e8f14 100644
--- a/src/include/catalog/pg_aggregate.dat
+++ b/src/include/catalog/pg_aggregate.dat
@@ -558,26 +558,28 @@
   aggfinalfn => 'array_agg_finalfn', aggcombinefn => 'array_agg_combine',
   aggserialfn => 'array_agg_serialize',
   aggdeserialfn => 'array_agg_deserialize', aggfinalextra => 't',
-  aggtranstype => 'internal' },
+  aggtranstype => 'internal', aggtransspace => '-1' },
 { aggfnoid => 'array_agg(anyarray)', aggtransfn => 'array_agg_array_transfn',
   aggfinalfn => 'array_agg_array_finalfn',
   aggcombinefn => 'array_agg_array_combine',
   aggserialfn => 'array_agg_array_serialize',
   aggdeserialfn => 'array_agg_array_deserialize', aggfinalextra => 't',
-  aggtranstype => 'internal' },
+  aggtranstype => 'internal', aggtransspace => '-1' },
 
 # text
 { aggfnoid => 'string_agg(text,text)', aggtransfn => 'string_agg_transfn',
   aggfinalfn => 'string_agg_finalfn', aggcombinefn => 'string_agg_combine',
   aggserialfn => 'string_agg_serialize',
-  aggdeserialfn => 'string_agg_deserialize', aggtranstype => 'internal' },
+  aggdeserialfn => 'string_agg_deserialize',
+  aggtranstype => 'internal', aggtransspace => '-1' },
 
 # bytea
 { aggfnoid => 'string_agg(bytea,bytea)',
   aggtransfn => 'bytea_string_agg_transfn',
   aggfinalfn => 'bytea_string_agg_finalfn',
   aggcombinefn => 'string_agg_combine', aggserialfn => 'string_agg_serialize',
-  aggdeserialfn => 'string_agg_deserialize', aggtranstype => 'internal' },
+  aggdeserialfn => 'string_agg_deserialize',
+  aggtranstype => 'internal', aggtransspace => '-1' },
 
 # range
 { aggfnoid => 'range_intersect_agg(anyrange)',
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 20bf9ea9cdf..a357e1d0c0e 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -1470,7 +1470,7 @@ WHERE aggfnoid = 0 OR aggtransfn = 0 OR
     (aggkind = 'n' AND aggnumdirectargs > 0) OR
     aggfinalmodify NOT IN ('r', 's', 'w') OR
     aggmfinalmodify NOT IN ('r', 's', 'w') OR
-    aggtranstype = 0 OR aggtransspace < 0 OR aggmtransspace < 0;
+    aggtranstype = 0 OR aggmtransspace < 0;
  ctid | aggfnoid 
 ------+----------
 (0 rows)
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 2fb3a852878..cd674d7dbca 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -847,7 +847,7 @@ WHERE aggfnoid = 0 OR aggtransfn = 0 OR
     (aggkind = 'n' AND aggnumdirectargs > 0) OR
     aggfinalmodify NOT IN ('r', 's', 'w') OR
     aggmfinalmodify NOT IN ('r', 's', 'w') OR
-    aggtranstype = 0 OR aggtransspace < 0 OR aggmtransspace < 0;
+    aggtranstype = 0 OR aggmtransspace < 0;
 
 -- Make sure the matching pg_proc entry is sensible, too.
 
-- 
2.39.5 (Apple Git-154)