Hi Shihao,

On Mon, 31 Aug 2026 at 10:21, shihao zhong <[email protected]> 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


Reply via email to