Horton

Re: Ignored join clause

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Gierth <andrew@tao11.riddles.org.uk>
Cc: Andreas Karlsson <andreas@proxel.se>, pgsql-bugs@lists.postgresql.org
Date: 2018-04-20T16:32:55Z
Lists: pgsql-bugs

Attachments

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

Commits

  1. Tweak a couple of planner APIs to save recalculating join relids.

  2. Change more places to be less trusting of RestrictInfo.is_pushed_down.

  3. Fix broken extract_actual_join_clauses call in 9.6 postgres_fdw.

  4. Fix incorrect handling of join clauses pushed into parameterized paths.

  5. Handle restriction clause lists more uniformly in postgres_fdw.