| From: | shihao zhong <zhong950419(at)gmail(dot)com> |
|---|---|
| To: | Andrei Lepikhov <lepihov(at)gmail(dot)com> |
| Cc: | Priyanka S <developerette(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [PATCH] Planner support function for generate_subscripts() |
| Date: | 2026-09-09 01:20:29 |
| Message-ID: | CAGRkXqTDfHXXDee3v7xaSihO0NYgECvED5pnXBb7YOEcB9sxgg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Add-a-planner-support-function-for-generate_subsc.patch | application/octet-stream | 21.2 KB |
| v3-0002-Lower-generate_subscripts-s-prorows-estimate-to-1.patch | application/octet-stream | 3.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2026-09-09 01:38:41 | Re: Assert failure in try_nestloop_path() |
| Previous Message | shihao zhong | 2026-09-09 01:13:53 | Re: REPACK (CONCURRENTLY) decoding worker is canceled by lock_timeout |