Re: Ignored join clause

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: Andreas Karlsson <andreas(at)proxel(dot)se>, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: Ignored join clause
Date: 2018-04-20 16:32:55
Message-ID: 27781.1524241975@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

I wrote:
> So the only practical answer seems to be to teach
> extract_actual_join_clauses to check the clause's syntactic level
> along with is_pushed_down, as per the attached patch.

After further thought about this, I decided that that patch didn't go
nearly far enough: in reality, just about every single place we test
RestrictInfo.is_pushed_down is potentially wrong in the same way that
extract_actual_join_clauses was, and needs to be taught to check the
clause relids in the same way. Hence the attached follow-on patch.
There might be a few of these places that don't really need the change
because they can never be reached while considering a parameterized join
path ... but I don't care to bet that that's true and will stay true.

There are a couple of places in analyzejoins.c that were already
examining required_relids, but were using bms_equal, which now seems
overly strict; this patch makes them use bms_subset like the other
places. Thoughts about that?

Also, I wondered in commit 306d6e59f whether changing the signature of
extract_actual_join_clauses in the back branches was really a good idea.
While it's still unpleasant, I'm inclined to leave it that way, because
any code that is using that function is almost certainly broken due to
this issue and needs to be fixed anyway. I see that the reason
postgres_fdw doesn't use that function as of v10 is commit 28b047875,
which probably should have been back-patched to 9.6 anyhow.

The patch as attached adds a couple of bms_union() steps to calculate join
relids in places that didn't have them handy. In both places this could
be avoided by passing down the join relids from a caller, but it'd require
API changes to globally-visible routines, so I thought eating some cycles
would be a safer solution in the back branches. We could clean that up
in HEAD though.

regards, tom lane

Attachment Content-Type Size
is-pushed-down-followup.patch text/x-diff 20.5 KB

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Greg Clough 2018-04-20 16:51:34 RE: BUG #15165: The application server could not be contacted
Previous Message PG Bug reporting form 2018-04-20 15:45:08 BUG #15165: The application server could not be contacted