On Fri, Dec 15, 2023 at 11:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Richard Guo <guofenglinux@gmail.com> writes: > On Thu, Dec 14, 2023 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Probably better to clamp tuple width estimates to MaxAllocSize. >> Anything larger would not correspond to reality anyhow.
> Fair point. How about the attached patch?
We'd need to hit at least build_joinrel_tlist too.
Right. And I think we also need to hit add_placeholders_to_joinrel.
Not sure offhand whether this is enough to cover upper-relation tlists.
I think we need to do the same to create_one_window_path. For intermediate WindowAggPaths, we need to add the WindowFuncs to their output targets, and that would increase the width of the tlists.
Also, it is possible that there could be tuple-width overflow occurring within function get_rel_data_width(). The return value of this function is used to calculate density, or returned by get_relation_data_width(). So I wonder if we could change the return type of get_rel_data_width() and get_relation_data_width() to be double, and handle the overflow in the callers if needed. But this may lead to ABI break.
As far as the specifics go, is it enough to clamp once? I think we'd either have to clamp after each addition, or change the running-sum variables to double and clamp just before storing into the final width field. The latter seems probably less error-prone in the long run.
Agreed.
Also, given that we'll need at least three copies of the clamp rule, I wonder if it's worth inventing a function comparable to clamp_row_est().
Yeah, I think we should do that.
Attached is an updated patch for all the changes. It also changes the loop_count parameter to be double for compute_bitmap_pages() in passing. It does not improve the comments for compute_bitmap_pages() though.
