On Wed, Feb 3, 2016 at 11:17 AM, Ashutosh Bapat
wrote:
> There seems to be double successive assignment to fdw_private in recent
> commit. Here's patch to remove the first one.
Committed along with a fix for another problem I noted along the way.
--
Robert Haas
EnterpriseDB: http://www.enterpri
There seems to be double successive assignment to fdw_private in recent
commit. Here's patch to remove the first one.
On Wed, Feb 3, 2016 at 7:40 PM, Robert Haas wrote:
> On Tue, Feb 2, 2016 at 10:42 PM, Corey Huinker
> wrote:
> >> I don't see how. There really is no declaration in there for a
On Tue, Feb 2, 2016 at 10:42 PM, Corey Huinker wrote:
>> I don't see how. There really is no declaration in there for a
>> variable called server.
>
> Absolutely correct. My only guess is that it was from failing to make clean
> after a checkout/re-checkout. A good reason to have even boring regr
>
>
> I don't see how. There really is no declaration in there for a
> variable called server.
>
Absolutely correct. My only guess is that it was from failing to make clean
after a checkout/re-checkout. A good reason to have even boring regression
tests.
Regression tests added.
diff --git a/cont
On Tue, Feb 2, 2016 at 5:57 PM, Corey Huinker wrote:
>> postgres_fdw.c:2642:16: error: use of undeclared identifier 'server'
>> foreach(lc, server->options)
>
>
> Odd. Compiled for me. Maybe I messed up the diff. Will get back to you on
> that with the tests suggested.
I d
>
>
> postgres_fdw.c:2642:16: error: use of undeclared identifier 'server'
> foreach(lc, server->options)
>
Odd. Compiled for me. Maybe I messed up the diff. Will get back to you on
that with the tests suggested.
> ^
> ../../src/includ
On Wed, Jan 27, 2016 at 11:19 PM, Corey Huinker wrote:
> The possible tests might be:
> - creating a server/table with fetch_size set
> - altering an existing server/table to have fetch_size set
> - verifying that the value exists in pg_foreign_server.srvoptions and
> pg_foreign_table.ftoptions.
>
Corey Huinker wrote:
> Enjoy.
Didn't actually look into the patch but looks like there's progress
here and this might be heading for commit. Moving to next one.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
> Looks pretty good. You seem to have added a blank line at the end of
> postgres_fdw.c, which should be stripped back out.
>
Done.
>
> > I'm not too keen on having *no* new regression tests, but defer to your
> > judgement.
>
> So... I'm not *opposed* to regression tests. I just don't see
On Mon, Jan 25, 2016 at 4:18 PM, Corey Huinker wrote:
> Revised in patch v3:
> * get_option() and get_fetch_size() removed, fetch_size searches added to
> existing loops.
> * Move fetch_size <= 0 tests into postgres_fdw_validator() routine in
> option.c
> * DEBUG1 message removed, never intended t
On Mon, Jan 25, 2016 at 3:22 PM, Robert Haas wrote:
> On Mon, Jan 25, 2016 at 1:21 PM, Corey Huinker
> wrote:
> >> - We could consider folding fetch_size into "Remote Execution
> >> Options", but maybe that's too clever.
> >
> > If you care to explain, I'm listening. Otherwise I'm going forward
On Mon, Jan 25, 2016 at 1:21 PM, Corey Huinker wrote:
>> - We could consider folding fetch_size into "Remote Execution
>> Options", but maybe that's too clever.
>
> If you care to explain, I'm listening. Otherwise I'm going forward with the
> other suggestions you've made.
It's just a little unfo
>
>
> Review:
>
> - There is an established idiom in postgres_fdw for how to extract
> data from lists of DefElems, and this patch does something else
> instead. Please make it look like postgresIsForeignRelUpdatable,
> postgresGetForeignRelSize, and postgresImportForeignSchema instead of
> invent
On Thu, Dec 31, 2015 at 1:10 AM, Corey Huinker wrote:
> Here's a re-based patch. Notable changes since the last patch are:
>
> - some changes had to move to postgres_fdw.h
> - the regression tests are in their own script fetch_size.sql /
> fetch_size.out. that should make things easier for the rev
>
>
>> I believe it won't be needed as a regression test but DEBUGn
>> messages could be usable to see "what was sent to the remote
>> side". It can be shown to the console so easily done in
>> regression test. See the deocumentation for client_min_messages
>> for details. (It could receive far man
>and fetch a number of rows that, by its estimation, would fit in the memory
>available
What's wrong with having size limit in the first place? It seems to
make much more sense.
Vladimir
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription
On Sat, Dec 26, 2015 at 6:16 AM, Vladimir Sitnikov <
sitnikov.vladi...@gmail.com> wrote:
> >Have you got numbers showing any actual performance win for postgres_fdw?
>
> For JDBC purposes, it would be nice to have an ability of asking
> backend "to stop fetching if produced more than X MiB of resp
>Have you got numbers showing any actual performance win for postgres_fdw?
For JDBC purposes, it would be nice to have an ability of asking
backend "to stop fetching if produced more than X MiB of response
data".
For small table (4 int4 fields), having decent fetchSize (~1000) makes
result process
On Fri, Dec 25, 2015 at 12:31 AM, Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp> wrote:
> Hello,
>
> At Thu, 24 Dec 2015 18:31:42 -0500, Corey Huinker
> wrote in fh+zueykccdu8p0pproyywlep5obrjkce5o7swqdf...@mail.gmail.com>
> > On Wed, Dec 23, 2015 at 3:08 PM, Jim Nasby
> wrote:
> >
> > >
Hello,
At Thu, 24 Dec 2015 18:31:42 -0500, Corey Huinker
wrote in
> On Wed, Dec 23, 2015 at 3:08 PM, Jim Nasby wrote:
>
> > On 12/23/15 12:15 PM, Corey Huinker wrote:
> >
> >> That's fair. I'm still at a loss for how to show that the fetch size was
> >> respected by the remote server, suggest
On Wed, Dec 23, 2015 at 3:08 PM, Jim Nasby wrote:
> On 12/23/15 12:15 PM, Corey Huinker wrote:
>
>> That's fair. I'm still at a loss for how to show that the fetch size was
>> respected by the remote server, suggestions welcome!
>>
>
> A combination of repeat() and generate_series()?
>
> I'm gues
On 12/23/15 12:15 PM, Corey Huinker wrote:
That's fair. I'm still at a loss for how to show that the fetch size was
respected by the remote server, suggestions welcome!
A combination of repeat() and generate_series()?
I'm guessing it's not that obvious and that I'm missing something; can
you
On Wed, Dec 23, 2015 at 8:43 AM, Michael Paquier
wrote:
> On Mon, Sep 21, 2015 at 1:52 PM, Corey Huinker
> wrote:
> > Attached is the patch / diff against current master.
>
> I have moved this patch to next CF, there is a new patch, but no reviews.
> --
> Michael
>
That's fair. I'm still at a
On Mon, Sep 21, 2015 at 1:52 PM, Corey Huinker wrote:
> Attached is the patch / diff against current master.
I have moved this patch to next CF, there is a new patch, but no reviews.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subsc
On Fri, Sep 4, 2015 at 2:45 AM, Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp> wrote:
> Hello,
>
> > > > @@ -915,6 +933,23 @@ postgresBeginForeignScan(ForeignScanState *node,
> > > int eflags)
> ...
> > > > + def = get_option(table->options, "fetch_size");
> >
> > > I don't think it's a
Hello,
> > > @@ -915,6 +933,23 @@ postgresBeginForeignScan(ForeignScanState *node,
> > int eflags)
...
> > > + def = get_option(table->options, "fetch_size");
>
> > I don't think it's a good idea to make such checks at runtime - and
> > either way it's somethign that should be reported back u
On Wed, Sep 2, 2015 at 9:08 AM, Andres Freund wrote:
> On 2015-02-27 13:50:22 -0500, Corey Huinker wrote:
> > +static DefElem*
> > +get_option(List *options, char *optname)
> > +{
> > + ListCell *lc;
> > +
> > + foreach(lc, options)
> > + {
> > + DefElem *def = (DefElem *)
On 2015-02-27 13:50:22 -0500, Corey Huinker wrote:
> +static DefElem*
> +get_option(List *options, char *optname)
> +{
> + ListCell *lc;
> +
> + foreach(lc, options)
> + {
> + DefElem *def = (DefElem *) lfirst(lc);
> +
> + if (strcmp(def->defname, optname) == 0)
Attached is a diff containing the original (working) patch from my
(incomplete) patch, plus regression test changes and documentation changes.
While it's easy to regression-test the persistence of the fetch_size
options, I am confounded as to how we would show that the fetch_size
setting was respe
Hello,
> Redshift has a table, stv_inflight, which serves about the same purpose as
> pg_stat_activity. Redshift seems to perform better with very high fetch
> sizes (10,000 is a good start), so the default foreign data wrapper didn't
> perform so well.
I agree with you.
> I was able to confirm
I applied this patch to REL9_4_STABLE, and I was able to connect to a
foreign database (redshift, actually).
the basic outline of the test is below, names changed to protect my
employment.
create extension if not exists postgres_fdw;
create server redshift_server foreign data wrapper postgres_fd
Hmm, somehow I removed some recipients, especially the
list. Sorry for the duplicate.
-
Sorry, I've been back. Thank you for the comment.
> Do you have any insight into where I would pass the custom row fetches from
> the table struct to the scan struct?
Yeah it's one simple way to tune it,
Last year I was working on a patch to postgres_fdw where the fetch_size
could be set at the table level and the server level.
I was able to get the settings parsed and they would show up in
pg_foreign_table
and pg_foreign_servers. Unfortunately, I'm not very familiar with how
foreign data wrappers
Thank you for the comment.
The automatic way to determin the fetch_size looks become too
much for the purpose. An example of non-automatic way is a new
foreign table option like 'fetch_size' but this exposes the
inside too much... Which do you think is preferable?
Thu, 22 Jan 2015 11:17:52 -0500,
Kyotaro HORIGUCHI writes:
> Hello, as the discuttion on async fetching on postgres_fdw, FETCH
> with data-size limitation would be useful to get memory usage
> stability of postgres_fdw.
> Is such a feature and syntax could be allowed to be added?
This seems like a lot of work, and frankly an in
35 matches
Mail list logo
