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