Re: [PATCH] postgres_fdw extension support

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Paul Ramsey <pramsey(at)cleverelephant(dot)ca>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: [PATCH] postgres_fdw extension support
Date: 2015-10-01 02:46:43
Message-ID: CAB7nPqSwRDWmCx--u8tL7XjVbTi9AeRKJ9J2mpWierjeqKxRdQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 1, 2015 at 7:57 AM, Paul Ramsey wrote:
> On September 30, 2015 at 3:32:21 PM, Michael Paquier wrote:
>
> OK. Once you can get a new patch done with a reworked
> extractExtensionList, I'll get a new look at it in a timely fashion
> and then let's move it to a committer's hands.

So, I had a final look at that, and finished with the attached. I have
done mainly cosmetic changes such as adjusting some error messages,
correcting the documentation that still referred to FDW supporting the
option "extensions", rewording things. Also, it seems to me that it
makes more sense to move extractExtensionList in option.c. You have as
well forgotten to remove a couple of things related to the previous
FDW interface:
- In InitializeShippableCache, there is no need to check for
FOREIGNDATAWRAPPEROID
- PgFdwRelationInfo does not need wrapper
- postgresGetForeignRelSize does not need to set up the wrapper OID by
calling GetForeignDataWrapper

I also had shared feelings about exposing PgFdwRelationInfo in
postgres_fdw.h, but your approach looks to be the cleanest way because
the extension list is available in the global context through
foreign_rel when checking the shippability of an expression. And I
guess that we definitely do not want to pass the extension list in a
bunch of routines, like appendWhereClause, is_foreign_expr,
classifyConditions, etc. That would be bug-prone thinking long-term.

This patch is switched as "ready for committer".
Regards,
--
Michael

Attachment Content-Type Size
20151001_postgres_fdw_extensions.patch application/x-patch 24.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2015-10-01 03:46:30 Re: LISTEN denial of service with aborted transaction
Previous Message Kouhei Kaigai 2015-10-01 02:15:29 Re: Foreign join pushdown vs EvalPlanQual