Re: [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(at)gmail(dot)com> wrote:

> Hi hackers,
> generate_subscripts() has no planner support function, so its row
> estimate is always the prorows value of 1000, no matter what the
> arguments are. unnest() has been estimating its row count from the
> array argument since v12. The attached 0001 does the same for
> generate_subscripts().
>
> One difference from unnest() is that generate_subscripts() returns
> one row per subscript of the requested dimension, not one row per
> element. An exact answer is therefore only possible when the array
> is a plan-time constant.
>
> The support function handles three cases:
>
> 1. If any argument is a constant NULL, it reports zero rows, since the
> function is strict.
> 2. If both the array and the dimension number are
> constants, it reports the exact length of that dimension.
> 3. If only the dimension number is known and it is 1, it uses
> estimate_array_length().
>
> That works because for one-dimensional arrays, the element count
> equals the length of dimension 1. In all other cases it declines and
> prorows applies as before.
>
> This can change plans for the better. Joining five subscripts
> against an indexed table:
> Hash Join (cost=637.00..649.63 rows=1000 width=45)
> Hash Cond: (s.s = items.id)
> -> Function Scan on generate_subscripts s (rows=1000) (actual rows=5)
> -> Hash
> -> Seq Scan on items (rows=20000)
> becomes
> Nested Loop (cost=0.29..41.58 rows=5 width=45)
> -> Function Scan on generate_subscripts s (rows=5) (actual rows=5)
> -> Index Scan using items_pkey on items
>
> A note on the statistics path. estimate_array_length() uses the
> DECHIST average, which counts distinct elements, so arrays with many
> duplicate or NULL elements get underestimated. unnest() behaves the
> same way. Fixing that centrally looks like separate work. The new
> regression tests use arrays of distinct elements to keep the expected
> output deterministic.
>
> 0002 lowers prorows from 1000 to 100. After 0001, prorows is only
> reached when the dimension number is unknown at plan time, or when a
> higher dimension of a non-constant array is requested. 100 matches
> what unnest() uses. I kept it as a separate patch so it can be taken
> or dropped on its own.
>
> CatVersion bump is required.
>
> Thanks,
> Shihao
>