Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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 >

Re: [HACKERS] [POC] FETCH limited by bytes.

> > > 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. > ^ >

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

> > > 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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

> > > 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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

> > > 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 >

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

> > >> 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

Re: [HACKERS] [POC] FETCH limited by bytes.

>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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

>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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

Hello, At Thu, 24 Dec 2015 18:31:42 -0500, Corey Huinker wrote in

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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) > > + { > > +

Re: [HACKERS] [POC] FETCH limited by bytes.

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)

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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 that

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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,

Re: [HACKERS] [POC] FETCH limited by bytes.

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

Re: [HACKERS] [POC] FETCH limited by bytes.

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

[HACKERS] [POC] FETCH limited by bytes.

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? == Postgres_fdw fetches tuples from remote servers using cursor. The transfer

Re: [HACKERS] [POC] FETCH limited by bytes.

Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp 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