On Fri, Jan 5, 2024 at 5:32 AM Andrei Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> On 2/1/2024 15:02, Alexander Korotkov wrote:
> > On Thu, Dec 28, 2023 at 6:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I'd say that that will break the cases rinfo_serial was introduced for.
> >> Now, I certainly don't love rinfo_serial and would be happier if we
> >> could do without it, but getting rid of it is another research project.
> >
> > It's a pity that no regression tests detect that. The attached patch
> > implements the special comparison function, which ignores the
> > 'rinfo_serial' field. This avoids marking 'rinfo_serial' as
> > pg_node_attr(equal_ignore).
> I've reviewed this patch, and it seems okay. It was a real blunder in
> our understanding of clauses; thanks to all for pointing this out and
> fixing it.
> The regression test shows where two fully equal join clauses, applied on
> different join levels, differ by the only required_relids. It covers the
> problem and can be helpful by itself. IMO, one line in this test is
> redundant (see attachment).
> The comment on the patch looks okay. But assuming required_relids and
> clause fields of two RestrinctInfos are equal, I can't imagine a
> situation to have different is_pushed_down and incompatible_relids. Is
> it really possible?
Andrei, thank you for your review. I've pushed it with your
suggestions and my further minor editings to comments and the commit
message. Sorry, I forgot to mention you as the reviewer.
One thing that worries me is that the patch version by Richard Guo [1]
marking 'rinfo_serial' as pg_node_attr(equal_ignore) didn't cause our
test suite to fail. Do you think you can write a test, which would
fail in that case?
Links
1. https://www.postgresql.org/message-id/CAMbWs48_EAPrc_C5qvK2WGvydHvht1JUnXwer9FVeU7t_zA%2BMQ%40mail.gmail.com
------
Regards,
Alexander Korotkov
