(2012/03/09 1:18), Tom Lane wrote: > I've been looking at this patch a little bit over the past day or so. > I'm pretty unhappy with deparse.c --- it seems like a real kluge, > inefficient and full of corner-case bugs. After some thought I believe > that you're ultimately going to have to abandon depending on ruleutils.c > for reverse-listing services, and it would be best to bite that bullet > now and rewrite this code from scratch.
Thanks for the review. Agreed to write own depraser for pgsql_fdw which handles nodes which can be pushed down. Every SQL-based FDW which constructs SQL statement for each local query would need such module inside. BTW, pgsql_fdw pushes only built-in objects which have no collation effect down to remote side, because user-defined objects might have different semantics on remote end. In future, such deparser will need some mechanism to map local object (or expression?) to remote one, like ROUTINE MAPPING, as discussed before. But it seems ok to assume that built-in objects have same name and semantics on remote end. > There are a couple of other points that make me think we need to revisit > the PlanForeignScan API definition some more, too. First, deparse.c is > far from cheap. While this doesn't matter greatly as long as there's > only one possible path for a foreign table, as soon as you want to > create more than one it's going to be annoying to do all that work N > times and then throw away N-1 of the results. Indeed deprase.c is not cheap, but I think that pgsql_fdw can avoid redundant works by deparsing SQL statement separately, unless we need to consider join-push-down. Possible parts are SELECT, FROM, WHERE and ORDER BY clauses. Simplest path uses SELECT and FROM, and other paths can be built by copying necessary clauses into individual buffers. Comments to the rest part are in my another reply to your recent post. Regards, -- Shigeru Hanada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
