Re: [PATCH] Planner support function for generate_subscripts() - Mailing list pgsql-hackers
| From | Priyanka S |
|---|---|
| Subject | Re: [PATCH] Planner support function for generate_subscripts() |
| Date | |
| Msg-id | CA+Db6gYaLov7zfWTgqgyp35YpQ3hs6EdxfC=1fveA3tC25ZR-A@mail.gmail.com Whole thread |
| List | pgsql-hackers |
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);
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
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.
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 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
pgsql-hackers by date: