On Wed, Nov 4, 2015 at 2:23 PM, Tom Lane wrote:
> Michael Paquier writes:
>> That's still strange to have a dummy object in
>> postgres_fdw.so just for testing purposes.
>
> We could drop the extra functions at the end of the test, but I don't
> see the point exactly. We'd just be leaving the re
Michael Paquier writes:
> On Wed, Nov 4, 2015 at 12:38 PM, Tom Lane wrote:
>> I had a possibly better idea: instead of manufacturing an empty extension
>> with a direct INSERT, hack on the one extension that we know for sure
>> will be installed, namely postgres_fdw itself. So we could do, eg,
>>
On Wed, Nov 4, 2015 at 12:38 PM, Tom Lane wrote:
> I wrote:
>> I left out the proposed regression tests because they fail in "make
>> installcheck" mode, unless you've previously built and installed cube
>> and seg, which seems like an unacceptable requirement to me. I don't
>> think that leaving
I wrote:
> I left out the proposed regression tests because they fail in "make
> installcheck" mode, unless you've previously built and installed cube
> and seg, which seems like an unacceptable requirement to me. I don't
> think that leaving the code untested is a good final answer, of course.
>
Thanks everyone for the held and feedback on this patch!
--
Paul Ramsey
http://cleverelephant.ca
http://postgis.net
On November 3, 2015 at 3:47:37 PM, Tom Lane (t...@sss.pgh.pa.us) wrote:
Robert Haas writes:
> On Tue, Nov 3, 2015 at 2:57 PM, Tom Lane wrote:
>> Paul Ramsey writes:
>>> [
Robert Haas writes:
> On Tue, Nov 3, 2015 at 2:57 PM, Tom Lane wrote:
>> Paul Ramsey writes:
>>> [ 20151006b_postgres_fdw_extensions.patch ]
>> There might be a case for raising a WARNING during
>> postgres_fdw_validator(), but no more than that, IMO. Certainly ERROR
>> during regular use of t
Paul Ramsey writes:
> [ 20151006b_postgres_fdw_extensions.patch ]
Starting to look through this now. I'm dubious of the decision to have
ExtractExtensionList throw errors if there are un-installed extensions
mentioned in the FDW options. Wouldn't it be a lot more convenient if
such extension na
On Tue, Nov 3, 2015 at 2:57 PM, Tom Lane wrote:
> Paul Ramsey writes:
>> [ 20151006b_postgres_fdw_extensions.patch ]
>
> Starting to look through this now. I'm dubious of the decision to have
> ExtractExtensionList throw errors if there are un-installed extensions
> mentioned in the FDW options.
On Tue, Oct 6, 2015 at 8:52 AM, Andres Freund wrote:
> I think it'd be good to add a test exercising two servers with different
> extensions marked as shippable.
Done,
P
20151006b_postgres_fdw_extensions.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@post
On 2015-10-06 07:01:53 -0700, Paul Ramsey wrote:
> diff --git a/contrib/postgres_fdw/sql/shippable.sql
> b/contrib/postgres_fdw/sql/shippable.sql
> new file mode 100644
> index 000..83ee38c
> --- /dev/null
> +++ b/contrib/postgres_fdw/sql/shippable.sql
> @@ -0,0 +1,76 @@
I think it'd be good
On Tue, Oct 6, 2015 at 10:32 PM, Andres Freund wrote:
> Maybe I'm missing something major here. But given that you're looking up
> solely based on Oid objnumber, Oid classnumber, how would this cache
> work if there's two foreign servers with different extension lists?
Oh. Nice catch here.
--
Mic
On Tue, Oct 6, 2015 at 6:55 AM, Andres Freund wrote:
> On 2015-10-06 06:42:17 -0700, Paul Ramsey wrote:
>> *sigh*, no you’re not missing anything. In moving to the cache and
>> marking things just as “shippable” I’ve lost the test that ensures
>> they are shippable for this *particular* server (wh
On 2015-10-06 06:42:17 -0700, Paul Ramsey wrote:
> *sigh*, no you’re not missing anything. In moving to the cache and
> marking things just as “shippable” I’ve lost the test that ensures
> they are shippable for this *particular* server (which only happens in
> the lookup stage). So yeah, the cache
On October 6, 2015 at 6:32:36 AM, Andres Freund
(and...@anarazel.de(mailto:and...@anarazel.de)) wrote:
> On 2015-10-06 06:28:34 -0700, Paul Ramsey wrote:
> > +/*
> > + * is_shippable
> > + * Is this object (proc/op/type) shippable to foreign server?
> > + * Check cache first, then look-up whether
On 2015-10-06 06:28:34 -0700, Paul Ramsey wrote:
> +/*
> + * is_shippable
> + * Is this object (proc/op/type) shippable to foreign server?
> + * Check cache first, then look-up whether (proc/op/type) is
> + * part of a declared extension if it is not cached.
> + */
> +bool
> +is_shippab
On Tue, Oct 6, 2015 at 5:32 AM, Andres Freund wrote:
> The problem is basically that cache invalidations can arrive while
> you're building new cache entries. Everytime you e.g. open a relation
> cache invalidations can arrive. Assume you'd written the code like:
> You're avoiding that by only e
On 2015-10-03 19:40:40 -0700, Paul Ramsey wrote:
> > > + /*
> > > + * Right now "shippability" is exclusively a function of whether
> > > + * the obj (proc/op/type) is in an extension declared by the user.
> > > + * In the future we could additionally have a whitelist of functions
> > > +
On October 4, 2015 at 9:56:10 PM, Michael Paquier
(michael.paqu...@gmail.com(mailto:michael.paqu...@gmail.com)) wrote:
> On Sun, Oct 4, 2015 at 11:40 AM, Paul Ramsey wrote:
> > I put all changes relative to your review here if you want a nice colorized
> > place to check
> >
> > https://github.c
On Sun, Oct 4, 2015 at 11:40 AM, Paul Ramsey wrote:
> I put all changes relative to your review here if you want a nice colorized
> place to check
>
> https://github.com/pramsey/postgres/commit/ed33e7489601e659f436d6afda3cce28304eba50
-/* updatable is available on both server and table */
Andres,
Thanks so much for the review!
I put all changes relative to your review here if you want a nice colorized
place to check
https://github.com/pramsey/postgres/commit/ed33e7489601e659f436d6afda3cce28304eba50
On October 3, 2015 at 8:49:04 AM, Andres Freund (and...@anarazel.de) wrote:
> +
Hi,
On 2015-10-01 11:46:43 +0900, Michael Paquier wrote:
> diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
> index 7547ec2..864bf53 100644
> --- a/contrib/postgres_fdw/option.c
> +++ b/contrib/postgres_fdw/option.c
> @@ -19,6 +19,8 @@
> #include "catalog/pg_foreign_tabl
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 h
On September 30, 2015 at 3:32:21 PM, Michael Paquier
(michael.paqu...@gmail.com) 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.
Done and thanks!
P
--
http://po
On Thu, Oct 1, 2015 at 4:33 AM, Paul Ramsey wrote:
>
>
> On September 30, 2015 at 7:06:58 AM, Tom Lane (t...@sss.pgh.pa.us) wrote:
>
> I wrote:
> > Hm. Wouldn't it be just fine if only the server is able to define a
> > list of extensions then? It seems to me that all the use-cases of this
> > fea
On September 30, 2015 at 7:06:58 AM, Tom Lane (t...@sss.pgh.pa.us) wrote:
Paul Ramsey writes:
> Hm. Wouldn't it be just fine if only the server is able to define a
> list of extensions then? It seems to me that all the use-cases of this
> feature require to have a list of extensions defined p
On September 30, 2015 at 12:54:44 AM, Michael Paquier
(michael.paqu...@gmail.com) wrote:
>> +extern bool extractExtensionList(char *extensionString,
>> + List **extensionOids);
>> What's the point of the boolean status in this new routine? The return
>> value of extractExtensionList is never
Paul Ramsey writes:
> Hm. Wouldn't it be just fine if only the server is able to define aÂ
> list of extensions then? It seems to me that all the use-cases of thisÂ
> feature require to have a list of extensions defined per server, andÂ
> not per fdw type. This would remove a level of complexit
On Mon, Sep 28, 2015 at 10:47 PM, Paul Ramsey wrote:
> On Sat, Sep 26, 2015 at 5:41 AM, Michael Paquier
> wrote:
>> On Sat, Sep 26, 2015 at 4:29 AM, Paul Ramsey wrote:
>>> On Thu, Aug 20, 2015 at 6:01 PM, Michael Paquier wrote:
>>> src/backend/utils/adt/format_type.c
>>> +/*
>>> + * This version
On Sat, Sep 26, 2015 at 5:41 AM, Michael Paquier
wrote:
> On Sat, Sep 26, 2015 at 4:29 AM, Paul Ramsey wrote:
>> On Thu, Aug 20, 2015 at 6:01 PM, Michael Paquier wrote:
>> src/backend/utils/adt/format_type.c
>> +/*
>> + * This version allows a nondefault typemod to be specified and fully
>> qualif
On Sat, Sep 26, 2015 at 4:29 AM, Paul Ramsey wrote:
> On Thu, Aug 20, 2015 at 6:01 PM, Michael Paquier wrote:
> src/backend/utils/adt/format_type.c
> +/*
> + * This version allows a nondefault typemod to be specified and fully
> qualified.
> + */
> +char *
> +format_type_with_typemod_qualified(Oid
Back from summer and conferencing, and finally responding, sorry for
the delay...
On Thu, Aug 20, 2015 at 6:01 PM, Michael Paquier
wrote:
>
>
> if (needlabel)
> appendStringInfo(buf, "::%s",
> -
> format_type_with_typemod(node->consttype,
> -
> node->consttypmod));
> +
> f
On Sat, Aug 22, 2015 at 12:55 AM, Alvaro Herrera
wrote:
> Michael Paquier wrote:
>
> > if (needlabel)
> > appendStringInfo(buf, "::%s",
> > -
> > format_type_with_typemod(node->consttype,
> > -
> > node->consttypmod));
> > +
> > format_type_be_qualified(node->consttype));
On Fri, Aug 21, 2015 at 12:55:39PM -0300, Alvaro Herrera wrote:
> Michael Paquier wrote:
>
> > if (needlabel)
> > appendStringInfo(buf, "::%s",
> > -
> > format_type_with_typemod(node->consttype,
> > -
> > node->consttypmod));
> > +
> > format_type_be_qualified(node->constt
Michael Paquier wrote:
> if (needlabel)
> appendStringInfo(buf, "::%s",
> -
> format_type_with_typemod(node->consttype,
> -
> node->consttypmod));
> +
> format_type_be_qualified(node->consttype));
> Pondering more about this one, I think that we are going to need a new
> ro
On Thu, Jul 23, 2015 at 11:48 PM, Paul Ramsey
wrote:
> On Wed, Jul 22, 2015 at 12:19 PM, Paul Ramsey
wrote:
>>
>> I’ll have a look at doing invalidation for the case of changes to the FDW
>> wrappers and servers.
>
> Here's an updated patch that clears the cache on changes to foreign
> wrappers a
Thanks so much Michael! Let me know when you have further feedback I should
incorporate.
ATB,
P.
--
http://postgis.net
http://cleverelephant.ca
On July 25, 2015 at 4:52:11 AM, Michael Paquier (michael.paqu...@gmail.com)
wrote:
On Sat, Jul 25, 2015 at 2:19 AM, Paul Ramsey wrote:
> On Th
On Sat, Jul 25, 2015 at 2:19 AM, Paul Ramsey wrote:
> On Thu, Jul 23, 2015 at 7:48 AM, Paul Ramsey
> wrote:
>> Here's an updated patch that clears the cache on changes to foreign
>> wrappers and servers.
>
> Any chance one of you folks could by my official commitfest reviewer?
> Appreciate all t
On Thu, Jul 23, 2015 at 7:48 AM, Paul Ramsey wrote:
> Here's an updated patch that clears the cache on changes to foreign
> wrappers and servers.
Any chance one of you folks could by my official commitfest reviewer?
Appreciate all the feedback so far!
https://commitfest.postgresql.org/6/304/
Th
On 2015-07-23 07:48:49 -0700, Paul Ramsey wrote:
> fdw=# ALTER SERVER foreign_server OPTIONS ( extensions 'postgis' );
> ALTER SERVER
> fdw=# ALTER SERVER foreign_server OPTIONS ( extensions 'postgis,seg' );
> ERROR: option "extensions" provided more than once
>
> Once set, an option seems to be
On Thu, Jul 23, 2015 at 7:48 AM, Paul Ramsey wrote:
> In testing it I came across an unrelated issue which could make it
> hard for users to manage the options on their wrappers/servers
>
> fdw=# ALTER SERVER foreign_server OPTIONS ( extensions 'postgis' );
> ALTER SERVER
> fdw=# ALTER SERVER for
On Wed, Jul 22, 2015 at 12:19 PM, Paul Ramsey wrote:
>
> I’ll have a look at doing invalidation for the case of changes to the FDW
> wrappers and servers.
Here's an updated patch that clears the cache on changes to foreign
wrappers and servers.
In testing it I came across an unrelated issue which
On Wed, Jul 22, 2015 at 2:55 PM, Tom Lane wrote:
> Robert Haas writes:
>> On Tue, Jul 21, 2015 at 2:27 PM, Andres Freund wrote:
>>> But I'm not going to complain too loudly if we don't do invalidation.
>
>> Not doing invalidation seems silly to me. But I don't want to bend
>> Paul too far aroun
On July 22, 2015 at 12:15:14 PM, Andres Freund (and...@anarazel.de) wrote:
It doesn't seem that unlikely that somebody does an ALTER SERVER OPTIONS
SET .. to add an extension to be shippable while connections are already
using the fdw. It'll be confusing if some clients are fast and some
others
On 2015-07-22 14:55:15 -0400, Tom Lane wrote:
> Just to be clear here: the case we are concerned about is, given that
> we have determined that function X is or is not a member of one of the
> extensions marked "shippable" for a given connection, is it likely that
> that status will change (while t
Robert Haas writes:
> On Tue, Jul 21, 2015 at 2:27 PM, Andres Freund wrote:
>> But I'm not going to complain too loudly if we don't do invalidation.
> Not doing invalidation seems silly to me. But I don't want to bend
> Paul too far around the axle, either.
Just to be clear here: the case we a
On Tue, Jul 21, 2015 at 2:27 PM, Andres Freund wrote:
> But I'm not going to complain too loudly if we don't do invalidation.
Not doing invalidation seems silly to me. But I don't want to bend
Paul too far around the axle, either.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The En
On Tue, Jul 21, 2015 at 11:29 AM, Paul Ramsey wrote:
> On July 21, 2015 at 11:22:12 AM, Tom Lane (t...@sss.pgh.pa.us) wrote:
>
> No, *not* populated first-time-through, because that won't handle any of
> the CREATE, DROP, or UPGRADE cases. It's also doing a lot of work you
> might never need. I wa
On July 21, 2015 at 11:22:12 AM, Tom Lane (t...@sss.pgh.pa.us) wrote:
> So: yes to a local cache of all forwardable functions/ops, populated in full
> the first time through (does that speak maybe to using a binary search on a
> sorted list instead of a hash, since I only pay the sort price once
On 2015-07-21 14:07:24 -0400, Tom Lane wrote:
> Paul Ramsey writes:
> > Folks are going to be OK w/ me dropping in new syscache entries so support
> > my niche little feature?
>
> No, mainly because it adds overhead without fixing your problem.
Meh. pg_extension updates are exceedingly rare, an
Paul Ramsey writes:
> On July 21, 2015 at 11:07:36 AM, Tom Lane (t...@sss.pgh.pa.us) wrote:
> I'm inclined to think that it's not really necessary to worry aboutÂ
> invalidating a per-connection cache of "is this function safe to ship"Â
> determinations.
> So: yes to a local cache of all forwar
On July 21, 2015 at 11:07:36 AM, Tom Lane (t...@sss.pgh.pa.us) wrote:
I'm inclined to think that it's not really necessary to worry about
invalidating a per-connection cache of "is this function safe to ship"
determinations.
So: yes to a local cache of all forwardable functions/ops, populated in
Paul Ramsey writes:
> Folks are going to be OK w/ me dropping in new syscache entries so support my
> niche little feature?
No, mainly because it adds overhead without fixing your problem. It's not
correct to suppose that a syscache on pg_extension would reliably report
anything; consider ALTER
On July 21, 2015 at 8:26:31 AM, Andres Freund
(and...@anarazel.de(mailto:and...@anarazel.de)) wrote:
> On 2015-07-21 17:00:51 +0200, Andres Freund wrote:
> > On 2015-07-21 07:55:17 -0700, Paul Ramsey wrote:
> > > On Tue, Jul 21, 2015 at 7:45 AM, Andres Freund wrote:
> > > So, right after reading
On 2015-07-21 17:00:51 +0200, Andres Freund wrote:
> On 2015-07-21 07:55:17 -0700, Paul Ramsey wrote:
> > On Tue, Jul 21, 2015 at 7:45 AM, Andres Freund wrote:
> > So, right after reading the options in postgresGetForeignRelSize,
> > expand the extension list into a list of all ops/functions, in a
On 2015-07-21 07:55:17 -0700, Paul Ramsey wrote:
> On Tue, Jul 21, 2015 at 7:45 AM, Andres Freund wrote:
> So, right after reading the options in postgresGetForeignRelSize,
> expand the extension list into a list of all ops/functions, in a
> sorted list, and let that carry through to the deparsing
On Tue, Jul 21, 2015 at 7:45 AM, Andres Freund wrote:
>> +
>> + /* We need this relation to scan */
>> + depRel = heap_open(DependRelationId, RowExclusiveLock);
>> +
>> + /* Scan the system dependency table for a all entries this operator */
>> + /* depends on, then iterate throug
Hi,
On 2015-07-21 07:28:22 -0700, Paul Ramsey wrote:
> /*
> @@ -229,6 +236,9 @@ foreign_expr_walker(Node *node,
> Oid collation;
> FDWCollateState state;
>
> + /* Access extension metadata from fpinfo on baserel */
> + PgFdwRelationInfo *fpinfo = (PgFdwRela
Here's a third revision that allows the 'extensions' option on the
wrapper as well, so that supported extensions can be declared once in
one place.
Since the "CREATE FOREIGN DATA WRAPPER" statement is actually called
inside the "CREATE EXTENSION" script for postgres_fdw, the way to get
this option
On July 17, 2015 at 5:57:42 AM, Simon Riggs
(si...@2ndquadrant.com(mailto:si...@2ndquadrant.com)) wrote:
> Options already exist on CREATE FOREIGN DATA WRAPPER, so it should be easy to
> support that.
>
> I'd rather add it once on the wrapper than be forced to list all the options
> on every
On 17 July 2015 at 13:51, Paul Ramsey wrote:
> There’s no facility to add OPTIONS to an EXTENSION right now, so this
> capability seems to be very much server-by-server (adding a FDW-specific
> capability to the EXTENSION mechanism seems like overkill for a niche
> feature addition).
>
Options
On July 17, 2015 at 12:49:04 AM, Simon Riggs
(si...@2ndquadrant.com(mailto:si...@2ndquadrant.com)) wrote:
> On 17 July 2015 at 01:23, Michael Paquier wrote:
>
> > > Well, as I see it there’s three broad categories of behavior available:
> > >
> > > 1- Forward nothing non-built-in (current beha
On 17 July 2015 at 01:23, Michael Paquier wrote:
> > Well, as I see it there’s three broad categories of behavior available:
> >
> > 1- Forward nothing non-built-in (current behavior)
> > 2- Use options to forward only specified non-built-in things (either in
> > function chunks (extensions, as
On Fri, Jul 17, 2015 at 1:08 AM, Paul Ramsey wrote:
> + if ( (!is_builtin(oe->opno)) &&
> (!is_in_extension(oe->opno, fpinfo)) )
> ... And this does not respect the project code format. See here for
> more details for example:
> http://www.postgresql.org/docs/devel/static/source.html
>
> I’m sorry,
Michael, thanks so much for the review!
On July 15, 2015 at 7:35:11 PM, Michael Paquier (michael.paqu...@gmail.com)
wrote:
This patch includes some diff noise, it would be better to remove that.
Done.
- if (!is_builtin(fe->funcid))
+ if (!is_builtin(fe->funcid) &&
(!is_in_extension(fe->funcid
On Thu, Jul 16, 2015 at 11:06 PM, Tom Lane wrote:
> Amit Langote writes:
>> On 2015-07-16 PM 12:43, Tom Lane wrote:
>>> The basic issue here is "how can a user control which functions/operators
>>> can be sent for remote execution?". While it's certainly true that
>>> sometimes you might want fu
Amit Langote writes:
> On 2015-07-16 PM 12:43, Tom Lane wrote:
>> The basic issue here is "how can a user control which functions/operators
>> can be sent for remote execution?". While it's certainly true that
>> sometimes you might want function-by-function control of that, Paul's
>> point was t
On 2015-07-16 PM 12:43, Tom Lane wrote:
>
> The basic issue here is "how can a user control which functions/operators
> can be sent for remote execution?". While it's certainly true that
> sometimes you might want function-by-function control of that, Paul's
> point was that extension-level granu
On Thu, Jul 16, 2015 at 12:43 PM, Tom Lane wrote:
> Michael Paquier writes:
>> On Thu, Jul 16, 2015 at 3:43 AM, Paul Ramsey
>> wrote:
>>> Attached is a patch that implements the extension support discussed at
>>> PgCon this year during the FDW unconference sesssion.
>
> ...
>
>> Thinking a bit
Michael Paquier writes:
> On Thu, Jul 16, 2015 at 3:43 AM, Paul Ramsey
> wrote:
>> Attached is a patch that implements the extension support discussed at
>> PgCon this year during the FDW unconference sesssion.
...
> Thinking a bit wider, why is this just limited to extensions?
The basic issu
On Thu, Jul 16, 2015 at 3:43 AM, Paul Ramsey wrote:
> Attached is a patch that implements the extension support discussed at
> PgCon this year during the FDW unconference sesssion. Highlights:
>
> * Pass extension operators and functions to the foreign server
> * Only send ops/funcs if the foreign
Hi all,
Attached is a patch that implements the extension support discussed at
PgCon this year during the FDW unconference sesssion. Highlights:
* Pass extension operators and functions to the foreign server
* Only send ops/funcs if the foreign server is declared to support the
relevant extension
71 matches
Mail list logo
