On 28/11/2023 01:37, Alexander Korotkov wrote:
> On Mon, Nov 27, 2023 at 8:07 PM Andres Freund <andres@anarazel.de> wrote:
Sorry for the late answer, I missed this thread because of vacation.
>> On 2023-11-27 11:29:48 +0530, Ashutosh Bapat wrote:
>>> How do we ensure that we are not making unnecessary copies of Bitmapsets?
>>
>> We don't - but that's not specific to this patch. Bitmapsets typically aren't
>> very large, I doubt that it's a significant proportion of the memory
>> usage. Adding refcounts or such would likely add more overhead than it'd save,
>> both in time and memory.
I'd already clashed with Tom on copying the required_relids field and
voluntarily made unnecessary copies in the project [1].
And ... stuck into huge memory consumption. The reason was in Bitmapsets:
When we have 1E3-1E4 partitions and try to reparameterize a join, one
bitmapset field can have a size of about 1kB. Having bitmapset
referencing Relation with a large index value, we had a lot of (for
example, 1E4 * 1kB) copies on each reparametrization of such a field.
Alexander Pyhalov should remember that case.
I don't claim we will certainly catch such an issue here, but it is a
reason why we should look at this option carefully.
>> I am a bit worried about the maintainability of remove_rel_from_query() et
>> al. Is there any infrastructure for detecting that some PlannerInfo field that
>> needs updating wasn't updated? There's not even a note in PlannerInfo that
>> documents that that needs to happen.
Thanks you for highlighting this issue.> That makes sense, thank you.
We need at least a comment about this.
> I'll write a patch adding this comment.
>
> BTW, what do you think about the patches upthread [1].
>
> Links
> 1. https://www.postgresql.org/message-id/CAPpHfdtLgCryACcrmLv=Koq9rAB3=tr5y9D84dGgvUhSCvjzjg@mail.gmail.com
0001 - Looks good and can be applied.
0002 - I am afraid the problems with expanded range table entries are
likewise described above. The patch makes sense, but it requires time to
reproduce corner cases. Maybe we can do it separately from the current
hotfix?
0003 - I think it is really what we need right now: SJE is quite a rare
optimization and executes before the entries expansion procedure. So it
looks less risky.
[1] Asymmetric partition-wise JOIN
https://www.postgresql.org/message-id/flat/CAOP8fzaVL_2SCJayLL9kj5pCA46PJOXXjuei6-3aFUV45j4LJQ%40mail.gmail.com
--
regards,
Andrei Lepikhov
Postgres Professional
