| From: | 韩建桥 <hanjianqiao(at)gmail(dot)com> |
|---|---|
| To: | shihao zhong <zhong950419(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [PATCH] Add row estimate tests for unnest() and integer generate_series() |
| Date: | 2026-09-01 07:04:29 |
| Message-ID: | CAPOa_Z=2rGt=pp6LMopmZQo5uNRjMF9ts=n-nQy1v8G6Mg=4fg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Shihao,
On Mon, 31 Aug 2026 at 10:21, shihao zhong <zhong950419(at)gmail(dot)com> wrote:
> The support functions for unnest() and the integer variants of
> generate_series() have no regression tests for their row estimates.
>
> They were added in v12, before planner_est.sql existed. The numeric
> and timestamp variants got tests together with their support
> functions.
>
> The attached patch adds sections for both, following the existing
> style. Test changes only.
This is my first patch review, so I picked this one because
it's tagged "Good First Review" -- thanks for a clean, well-scoped
patch to start with.
I reviewed and tested this patch with my Ubuntu 26.04 LTS Desktop.
Applied cleanly against the current master (3b120b1e94dd,
Tue Sep 1 11:42:13 2026 +0900). Full rebuild produced no compiler
warnings, and planner_est passes with the expected output exactly
matching a real run.
I also manually reviewed the new test cases and their expected
output against the SupportRequestRows behavior:
1. generate_series(1, 25) -> 25 rows.
2. generate_series(1,25,2) -> 13 rows: (25 - 1) / 2 + 1 = 13.
3. generate_series(25,1,-1) -> 25 rows descending.
4. generate_series(25,1) -> 0 row.
5. generate_series(1, NULL::int4) -> 0 row.
6. generate_series(1::int8, 25::int8, 3::int8) -> 9 rows: (25 - 1) / 3 + 1 = 9.
7. generate_series(1, 25, 0) -> Error, explain_mask_costs returns
default prorows value: 1000.
8. generate_series(1::int8, 10000000000::int8) -> 10000000000.
9. unnest('{1,2,3,4,5}'::int[]) -> 5 rows.
10. unnest('{{1,2,3},{4,5,6}}'::int[]) -> 6 rows, expand all elements by
row-major order.
11. unnest('{}'::int[]) -> 0 row.
12. unnest(NULL::int[]) -> 0 row.
13. unnest(ARRAY[1, 2, (SELECT 3)]) -> 3 rows, (SELECT 3) is 1 row.
14. unnest_table_1 case: arrays are all exactly 7 distinct elements,
so the DECHIST-based estimate of 7 per row (100 x 7 = 700).
15. unnest_table_2 case: only "id" is analyzed, so the array column
has no stats and the default estimate is 10 from function:
"estimate_array_length(...)". So the estimate is 5 x 10 = 50 and
15 rows actually.
Alias naming style makes sense: 'g(s)' for the extended
generate_series() cases,
and 'u(e)' for new unnest() cases - 'u' for unnest, 'e' for elements.
This looks good to me.
--
Regards,
hanjianqiao
| From | Date | Subject | |
|---|---|---|---|
| Next Message | JoongHyuk Shin | 2026-09-01 07:04:49 | Re: Deadlock detector fails to activate on a hot standby replica |
| Previous Message | Ashutosh Bapat | 2026-09-01 07:01:27 | Re: PGQ catalog representation and pg_dump support |