[PATCH] Planner support function for generate_subscripts()

Hi,

I've applied the patch and done some debugging to understand the behaviour.
The approach looks good and all the cases work correctly for me. Please
find below some minor review comments. This is my first code review for
postgres, so please excuse me if I have given any wrong inputs.

1) It would be good to add a line to the description saying that
generate_subscripts() returns 0 for dimensions which do not exist in the
array. This seems like an important case which should be mentioned
specifically.

2) Missing NULL-initialisation of these vars, plus unnecessary newlines.

File: src/backend/utils/adt/arrayfuncs.c
+                       Node       *arg1,
+                                          *arg2,
+                                          *arg3;

3) Would be good to change the comment to something more descriptive like
'Check that this is a FuncExpr'. Or delete it, whichever you prefer.

File: src/backend/utils/adt/arrayfuncs.c

+ if (is_funcclause(req->node)) /* be paranoid */

4) The declaration and assignment should be combined into a single line.

File: src/backend/utils/adt/arrayfuncs.c
+                                       ArrayType  *arr;
+                                       ...
+                                       arr = DatumGetArrayTypeP(((Const *)
arg1)->constvalue);

to

+ ArrayType *arr =
DatumGetArrayTypeP(((Const *) arg1)->constvalue);

5) Could you please add a test case for a dimension greater than 2? I've
tested this patch using a 4-dimensional array and it works. Here are my
query outputs, if you'd like to use them.

pgsql=# explain SELECT * FROM
generate_subscripts('{{{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}}},
{{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}}}}'::int[],
1);
QUERY PLAN
------------------------------------------------------------------------
Function Scan on generate_subscripts (cost=0.00..0.02 rows=2 width=4)
(1 row)

pgsql=# explain SELECT * FROM
generate_subscripts('{{{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}}},
{{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}}}}'::int[],
2);
QUERY PLAN
------------------------------------------------------------------------
Function Scan on generate_subscripts (cost=0.00..0.03 rows=3 width=4)
(1 row)

pgsql=# explain SELECT * FROM
generate_subscripts('{{{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}}},
{{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}}}}'::int[],
3);
QUERY PLAN
------------------------------------------------------------------------
Function Scan on generate_subscripts (cost=0.00..0.04 rows=4 width=4)
(1 row)

pgsql=# explain SELECT * FROM
generate_subscripts('{{{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}}},
{{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}},{{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5},{1,2,3,4,5}}}}'::int[],
4);
QUERY PLAN
------------------------------------------------------------------------
Function Scan on generate_subscripts (cost=0.00..0.05 rows=5 width=4)
(1 row)

6) I debugged estimate_array_length() to understand the non-const array
case. For multi-dimensional arrays, it seems to use a default value of 10
to match a variable 'scalararraysel'. I was wondering if
generate_subscripts() and unnest() prorows should match this default value
of 10 instead of being set to 100?

Thanks & regards,
Priyanka

On Mon, Aug 31, 2026 at 7:34 AM shihao zhong <zhong950419@gmail.com> wrote:

Hi Priyanka,

Thanks for the review, and for testing the patch yourself. v2 is
attached.

1) This is already in the docs. The text below the function table in
func-srf.sgml says: "Zero rows are returned for arrays that do not have
the requested dimension, or if any input is NULL." The entries in the
table itself are kept to a line or two, so I left it where it is.

2) I set arg3 to NULL. It is the only one that is not always given a
value. arg1 and arg2 are set on the next line, so setting them to NULL
first would be a wasted store. I did join their declarations and their
assignments, as you suggest in 4).

3) I left that comment as it is. The same comment sits on the same
test in six other support functions, so it is how we usually write it.

4) I kept those two lines apart. As one line it would be 87 columns
wide, which is too long. The comment also makes more sense right above
the assignment than above the declaration.

5) Added. I used a 3-D array where all three dimensions have a
different length, so a mistake in the dimension indexing could not
still print the right number. I used one case instead of four, since
they all run the same line of code. While there I dropped one of the
old cases: two of them checked the same rule, that we give up for a
dimension above the first when the array is not a constant.

6) You are right. I think the reason is even better than the one you
give.

prorows is only used when the support function gives up.
array_unnest_support() never gives up: it sets req->rows every time it
is asked. So unnest() never uses its prorows of 100. For an array it
knows nothing about, unnest() really estimates 10, which is the default
in estimate_array_length(). My reason for picking 100 was to match
unnest(), but I was matching a number that is never read.

v2-0002 uses 10 instead. Two cases still fall back on prorows, and 10
fits both:

* The dimension number is not known when we plan. At run time it is
almost always 1, and 10 is what the dimension-1 case gives for an
array with no statistics.

* A dimension above the first is asked for on an array that is not a
constant. One dimension of an array is never longer than the whole
array, so a large number is wrong here.

1000 fits neither case.

I kept 0002 as its own patch. It only changes a number in the catalog,
and 0001 does not need it. If people do not agree on the number, 0001
can still go in.

Thanks,
Shihao

On 08/09/2026 03:21, shihao zhong wrote:

* A dimension above the first is asked for on an array that is not a
  constant.  One dimension of an array is never longer than the whole
  array, so a large number is wrong here.

1000 fits neither case.

I kept 0002 as its own patch.  It only changes a number in the catalog,
and 0001 does not need it.  If people do not agree on the number, 0001
can still go in.

I'd say that detecting the estimate_array_length's default value for the rows
number and returning constant 1000 as used before might make life of engineers
easier and query plans more stable during upgrade.
Hence, maybe it makes sense to follow the way of group-by estimations -
introduce something like one more 'isdefault' parameter to the
estimate_array_length?

Basically, this code looks good. But I'd like to see more assertions: if
something will be changed at the functions itself it would more quickly detect
issues in the prosupport routine. For example:
- Assert(req->root != NULL);
- Type of arg2
- Potentially, no more than 3 arguments of the function
- Maybe hard oid check on F_GENERATE_SUBSCRIPTS / F_GENERATE_SUBSCRIPTS_NODIR

This topic also raises the question of the practical usability of the average
total number of elements in an array column. Your tests highlight a gap in
estimates for more than two dimensions.

regards, Andrei Lepikhov,
pgEdge

Hi Andrei,
Thanks for the review. v3 attached.

But I'd like to see more assertions

I took all of these. One small change on the root check. The
comment above get_function_rows() says root can be NULL. So the
function now declines when root is NULL instead of asserting.
Without this check, estimate_expression_value() would crash.
The function also declines when req->funcid is not one of the two
generate_subscripts() entries, or when the call has fewer than two
arguments. These are runtime checks on purpose. Assertions go away
in production builds, and ALTER FUNCTION ... SUPPORT can attach this
function to anything. Once the funcid is checked, the parser
guarantees the argument count and types. So those two stay as plain
assertions.

maybe it makes sense to follow the way of group-by estimations -
introduce something like one more 'isdefault' parameter to the
estimate_array_length?

After 0002, the fallback and the default guess are both 10. So
detecting the default would not change any plan. If you would
rather keep prorows at 1000, an isdefault flag would be the right
tool, and I can rework 0002 that way. I still prefer 10. unnest()
has given 10 for an array with no statistics since v12, and matching
it keeps the two functions consistent.

Your tests highlight a gap in estimates for more than two
dimensions.

Yes. There are no statistics about single dimensions. The DECHIST
number is only an average of distinct element counts. So a higher
dimension of a non-constant array cannot be estimated today. Making
array_typanalyze collect such data would be a separate project.
Until then the patch declines instead of guessing.

Thanks,
Shihao

Hi,

v3 broke the CompilerWarnings CI task. v4 attached fixes that; nothing
else has changed.

Thanks,
Shihao