On Fri, 2023-03-24 at 16:41 -0400, Tom Lane wrote: > Christoph Berg <m...@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
