On Fri, 2023-03-24 at 16:41 -0400, Tom Lane wrote:
> Christoph Berg <myon@debian.org> writes:
> > Re: Tom Lane
> > > I don't actually see why a postgres_fdw test case is needed at all?
> > > The tests in explain.sql seem sufficient.
>
> > When I asked Laurenz about that he told me that local tables wouldn't
> > exercise the code specific for EXEC_FLAG_EXPLAIN_GENERIC.
>
> But there isn't any ... or at least, I fail to see what isn't sufficiently
> exercised by that new explain.sql test case that's identical to this one
> except for being a non-foreign table. Perhaps at some point this patch
> modified postgres_fdw code? But it doesn't now.
>
> I don't mind having a postgres_fdw test if there's something for it
> to test, but it just looks duplicative to me. Other things being
> equal, I'd prefer to test this feature in explain.sql, since (a) it's
> a core feature and (b) the core tests are better parallelized than the
> contrib tests, so the same test should be cheaper to run.
>
> > (Admittedly my knowledge of the planner wasn't deep enough to verify
> > that. Laurenz is currently traveling, so I don't know if he could
> > answer this himself now.)
>
> OK, thanks for the status update. What I'll do to get this off my
> plate is to push the patch without the postgres_fdw test -- if
> Laurenz wants to advocate for that when he returns, we can discuss it
> more.
Thanks for committing this.
As Chrisoph mentioned, I found that I could not reproduce the problem
that was addressed by the EXEC_FLAG_EXPLAIN_GENERIC hack using local
partitioned tables. My optimizer knowledge is not deep enough to tell
why, and it might well be a bug lurking in the FDW part of the
optimizer code. It is not FDW specific, since I discovered it with
oracle_fdw and could reproduce it with postgres_fdw.
I was aware that it is awkward to add a test to a contrib module, but
I thought that I should add a test that exercises the new code path.
But I am fine without the postgres_fdw test.
Yours,
Laurenz Albe
