Re: ORDER BY pushdowns seem broken in postgres_fdw

Ronan Dunklau <ronan.dunklau@aiven.io> writes:
> [ v8-0001-Fix-orderby-handling-in-postgres_fdw.patch ]

I looked through this patch.  It's going in the right direction,
but I have a couple of nitpicks:

1. There are still some more places that aren't checking shippability
of the relevant opfamily.

2. The existing usage of find_em_expr_for_rel is fundamentally broken,
because that function will seize on the first EC member that is from the
given rel, whether it's shippable or not.  There might be another one
later that is shippable, so this is just the wrong API.  It's not like
this function gives us any useful isolation from the details of ECs,
because postgres_fdw is already looking into those elsewhere, notably
in find_em_expr_for_input_target --- which has the same order-sensitivity
bug.

I think that instead of doubling down on a wrong API, we should just
take that out and move the logic into postgres_fdw.c.  This also has
the advantage of producing a patch that's much safer to backpatch,
because it doesn't rely on the core backend getting updated before
postgres_fdw.so is.

So hacking on those two points, and doing some additional cleanup,
led me to the attached v9.  (In this patch, the removal of code
from equivclass.c is only meant to be applied to HEAD; we have to
leave the function in place in the back branches for API stability.)

If no objections, I think this is committable.

			regards, tom lane