On 4/10/2023 07:12, Alexander Korotkov wrote:
Hi!
Thanks for the review!

I think this is a neat optimization.

On Tue, Sep 12, 2023 at 4:58 PM Andrey Lepikhov
<a.lepik...@postgrespro.ru> wrote:
On 5/7/2023 21:28, Andrey Lepikhov wrote:
During the significant code revision in v.41 I lost some replacement
operations. Here is the fix and extra tests to check this in the future.
Also, Tom added the JoinDomain structure five months ago, and I added
code to replace relids for that place too.
One more thing, I found out that we didn't replace SJs, defined by
baserestrictinfos if no one self-join clause have existed for the join.
Now, it is fixed, and the test has been added.
To understand changes readily, see the delta file in the attachment.
Here is new patch in attachment. Rebased on current master and some
minor gaffes are fixed.

I went through the thread and I think the patch gets better shape.  A
couple of notes from my side.
1) Why replace_relid() makes a copy of lids only on insert/replace of
a member, but performs deletion in-place?

Shortly speaking, it was done according to the 'Paranoid' strategy.
The main reason for copying before deletion was the case with the rinfo required_relids and clause_relids. They both point to the same Bitmapset in some cases. And we feared such things for other fields. Right now, it may be redundant because we resolved the issue mentioned above in replace_varno_walker.

Relid replacement machinery is the most contradictory code here. We used a utilitarian approach and implemented a simplistic variant.

2) It would be nice to skip the insertion of IS NOT NULL checks when
they are not necessary.  [1] points that infrastructure from [2] might
be useful.  The patchset from [2] seems committed mow.  However, I
can't see it is directly helpful in this matter.  Could we just skip
adding IS NOT NULL clause for the columns, that have
pg_attribute.attnotnull set?
Thanks for the links, I will look into that case.

Links
1. https://www.postgresql.org/message-id/2375492.jE0xQCEvom%40aivenronan
2.  https://www.postgresql.org/message-id/flat/830269.1656693747%40sss.pgh.pa.us

--
regards,
Andrey Lepikhov
Postgres Professional



Reply via email to