Thread

1. Fwd: sql/med review - problems with patching

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-07-14T06:15:32Z

   Hello
   
   please, can you refresh patch, please?
   
   [pavel@nemesis pgsql]$ patch -p1 < backend.patch
   patching file src/backend/access/common/reloptions.c
   patching file src/backend/catalog/Makefile
   patching file src/backend/catalog/aclchk.c
   patching file src/backend/catalog/dependency.c
   patching file src/backend/catalog/heap.c
   patching file src/backend/catalog/system_views.sql
   patching file src/backend/commands/alter.c
   patching file src/backend/commands/analyze.c
   patching file src/backend/commands/comment.c
   patching file src/backend/commands/copy.c
   patching file src/backend/commands/discard.c
   patching file src/backend/commands/explain.c
   patching file src/backend/commands/foreigncmds.c
   patching file src/backend/commands/lockcmds.c
   patching file src/backend/commands/tablecmds.c
   Hunk #6 FAILED at 1980.
   Hunk #25 succeeded at 7190 (offset 3 lines).
   Hunk #26 succeeded at 7795 (offset 3 lines).
   Hunk #27 succeeded at 7812 (offset 3 lines).
   Hunk #28 succeeded at 7843 (offset 3 lines).
   1 out of 28 hunks FAILED -- saving rejects to file
   src/backend/commands/tablecmds.c.rej
   patching file src/backend/commands/vacuum.c
   patching file src/backend/executor/Makefile
   patching file src/backend/executor/execAmi.c
   
   
   *** src/backend/commands/tablecmds.c
   --- src/backend/commands/tablecmds.c
   *************** renameatt(Oid myrelid,
   *** 1980,1989 ****
   -- 1993,2003 ----
   -><------>ereport(ERROR,
    <----><------><------><------>(errcode(ERRCODE_WRONG_OBJECT_TYPE),
   ! <----><------><------><------> errmsg("\"%s\" is not a table, view,
   composite type,index or foreign table", <<<<< missing space before
   index
    <----><------><------><------><------><------>RelationGetRelationName(targetrelation))));
   
   is there a some special reason to divide diff to separate parts?
   
   I can't to compile code
   
   execAmi.c: In function ‘ExecReScan’:
   execAmi.c:186: error: ‘exprCtxt’ undeclared (first use in this function)
   execAmi.c:186: error: (Each undeclared identifier is reported only once
   execAmi.c:186: error: for each function it appears in.)
   make[3]: *** [execAmi.o] Error 1
   make[3]: Leaving directory `/home/pavel/src/pgsq
   
   because Tom commited significant changes in executor
   
   http://archives.postgresql.org/pgsql-committers/2010-07/msg00155.php
   
   When I looked to documentation I miss a some tutorial for foreign
   tables. There are only reference. I miss some paragraph where is
   cleanly and simple specified what is possible now and whot isn't
   possible. Enhancing of dblink isn't documented
   
   Why you don't use PQescapeLiteral for escaping. Isn't
   escape_param_str redundant and unsecure?
   
   In function  pgIterate(ForeignScanState *scanstate) you are iterare
   via pg result. I am thinking so using a cursor and fetching multiple
   rows should be preferable.
   
   Regards
   
   Pavel Stehule
   
   

2. Re: sql/med review - problems with patching

   Itagaki Takahiro <itagaki.takahiro@gmail.com> — 2010-07-20T08:12:28Z

   2010/7/14 Pavel Stehule <pavel.stehule@gmail.com>:
   > please, can you refresh patch, please?
   
   Updated patch attached. The latest version is always in the git repo.
   http://repo.or.cz/w/pgsql-fdw.git   (branch: fdw)
   I'm developing the patch on postgres' git repo. So, regression test
   for dblink might fail because of out-of-sync issue between cvs and git.
   
   > When I looked to documentation I miss a some tutorial for foreign
   > tables. There are only reference. I miss some paragraph where is
   > cleanly and simple specified what is possible now and whot isn't
   > possible. Enhancing of dblink isn't documented
   
   Sure. I'll start to write documentation when we agree the design of FDW.
   
   > In function  pgIterate(ForeignScanState *scanstate) you are iterare
   > via pg result. I am thinking so using a cursor and fetching multiple
   > rows should be preferable.
   
   Sure, but I'm thinking that it will be improved after libpq supports
   protocol-level cursor. The libpq improvement will be applied
   much more applications including postgresql_fdw.
   
   -- 
   Itagaki Takahiro
   

3. Re: sql/med review - problems with patching

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-07-20T09:40:18Z

   2010/7/20 Itagaki Takahiro <itagaki.takahiro@gmail.com>:
   > 2010/7/14 Pavel Stehule <pavel.stehule@gmail.com>:
   >> please, can you refresh patch, please?
   >
   > Updated patch attached. The latest version is always in the git repo.
   > http://repo.or.cz/w/pgsql-fdw.git   (branch: fdw)
   > I'm developing the patch on postgres' git repo. So, regression test
   > for dblink might fail because of out-of-sync issue between cvs and git.
   >
   >> When I looked to documentation I miss a some tutorial for foreign
   >> tables. There are only reference. I miss some paragraph where is
   >> cleanly and simple specified what is possible now and whot isn't
   >> possible. Enhancing of dblink isn't documented
   >
   > Sure. I'll start to write documentation when we agree the design of FDW.
   >
   >> In function  pgIterate(ForeignScanState *scanstate) you are iterare
   >> via pg result. I am thinking so using a cursor and fetching multiple
   >> rows should be preferable.
   >
   > Sure, but I'm thinking that it will be improved after libpq supports
   > protocol-level cursor. The libpq improvement will be applied
   > much more applications including postgresql_fdw.
   >
   
   is there some time frame for this task - or ToDo point? Minimally it
   has to be documented, because it can be a issue on larger sets -
   speed, memory usage. I am afraid about speed for queries like
   
   select * from large_dblink_tab limit 100;
   
   Regards
   
   Pavel
   
   > --
   > Itagaki Takahiro
   >
   
   

4. Re: sql/med review - problems with patching

   David Fetter <david@fetter.org> — 2010-07-20T14:55:55Z

   On Tue, Jul 20, 2010 at 11:40:18AM +0200, Pavel Stehule wrote:
   > 2010/7/20 Itagaki Takahiro <itagaki.takahiro@gmail.com>:
   > > 2010/7/14 Pavel Stehule <pavel.stehule@gmail.com>:
   > >> please, can you refresh patch, please?
   > >
   > > Updated patch attached. The latest version is always in the git
   > > repo.  http://repo.or.cz/w/pgsql-fdw.git   (branch: fdw) I'm
   > > developing the patch on postgres' git repo. So, regression test
   > > for dblink might fail because of out-of-sync issue between cvs and
   > > git.
   > >
   > >> When I looked to documentation I miss a some tutorial for foreign
   > >> tables. There are only reference. I miss some paragraph where is
   > >> cleanly and simple specified what is possible now and whot isn't
   > >> possible. Enhancing of dblink isn't documented
   > >
   > > Sure. I'll start to write documentation when we agree the design
   > > of FDW.
   > >
   > >> In function  pgIterate(ForeignScanState *scanstate) you are
   > >> iterare via pg result. I am thinking so using a cursor and
   > >> fetching multiple rows should be preferable.
   > >
   > > Sure, but I'm thinking that it will be improved after libpq
   > > supports protocol-level cursor. The libpq improvement will be
   > > applied much more applications including postgresql_fdw.
   > >
   > 
   > is there some time frame for this task - or ToDo point? Minimally it
   > has to be documented, because it can be a issue on larger sets -
   > speed, memory usage. I am afraid about speed for queries like
   > 
   > select * from large_dblink_tab limit 100;
   
   The general issue of passing qualifiers to the remote data source is
   complex, especially when the DML for that data source is different
   from PostgreSQL's DML.
   
   Do you have some ideas as to how to solve this problem in general?  In
   this case?
   
   Cheers,
   David.
   -- 
   David Fetter <david@fetter.org> http://fetter.org/
   Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
   Skype: davidfetter      XMPP: david.fetter@gmail.com
   iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
   
   Remember to vote!
   Consider donating to Postgres: http://www.postgresql.org/about/donate
   
   

5. Re: sql/med review - problems with patching

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-07-20T15:28:00Z

   2010/7/20 David Fetter <david@fetter.org>:
   > On Tue, Jul 20, 2010 at 11:40:18AM +0200, Pavel Stehule wrote:
   >> 2010/7/20 Itagaki Takahiro <itagaki.takahiro@gmail.com>:
   >> > 2010/7/14 Pavel Stehule <pavel.stehule@gmail.com>:
   >> >> please, can you refresh patch, please?
   >> >
   >> > Updated patch attached. The latest version is always in the git
   >> > repo.  http://repo.or.cz/w/pgsql-fdw.git   (branch: fdw) I'm
   >> > developing the patch on postgres' git repo. So, regression test
   >> > for dblink might fail because of out-of-sync issue between cvs and
   >> > git.
   >> >
   >> >> When I looked to documentation I miss a some tutorial for foreign
   >> >> tables. There are only reference. I miss some paragraph where is
   >> >> cleanly and simple specified what is possible now and whot isn't
   >> >> possible. Enhancing of dblink isn't documented
   >> >
   >> > Sure. I'll start to write documentation when we agree the design
   >> > of FDW.
   >> >
   >> >> In function  pgIterate(ForeignScanState *scanstate) you are
   >> >> iterare via pg result. I am thinking so using a cursor and
   >> >> fetching multiple rows should be preferable.
   >> >
   >> > Sure, but I'm thinking that it will be improved after libpq
   >> > supports protocol-level cursor. The libpq improvement will be
   >> > applied much more applications including postgresql_fdw.
   >> >
   >>
   >> is there some time frame for this task - or ToDo point? Minimally it
   >> has to be documented, because it can be a issue on larger sets -
   >> speed, memory usage. I am afraid about speed for queries like
   >>
   >> select * from large_dblink_tab limit 100;
   >
   > The general issue of passing qualifiers to the remote data source is
   > complex, especially when the DML for that data source is different
   > from PostgreSQL's DML.
   >
   > Do you have some ideas as to how to solve this problem in general?  In
   > this case?
   
   yes, I can. I expect so you can read from foreign table row by row,
   because it is supported by executor - via ExecForeignScan().
   
   dblink exec is called inside this method and result is stored.
   Repeated call of this method means repeated reading from tuplestore
   storage. So outer limit hasn't effect on loaded data - full result is
   fetched. I propose little bit different strategy. Using a cursor and
   repeating fetching of n rows. Plpgsql "FOR" statement uses 50 rows.
   For external tables 1000 can be enough. With this strategy max (n - 1)
   rows are fetched uselessly. So we don't need a local tuplestore and we
   will have a result early - with LIMIT clause.
   
   so
   ExecInitForeignScan()
   --> open cursor
   
   ExecForeignScan()
   --> if there are some fetched rows, return row
   --> if not, fetch 1000 rows
   
   ExecEndScan()
   --> close cursor
   
   Regards
   
   Pavel Stehule
   
   same mechanism is used in plpgsql.
   
   
   
   >
   > Cheers,
   > David.
   > --
   > David Fetter <david@fetter.org> http://fetter.org/
   > Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
   > Skype: davidfetter      XMPP: david.fetter@gmail.com
   > iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
   >
   > Remember to vote!
   > Consider donating to Postgres: http://www.postgresql.org/about/donate
   >
   
   

6. Re: sql/med review - problems with patching

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-07-21T15:49:51Z

   Hello
   
   I am playing with foreign tables now.
   
   I found a few small issues now:
   
   * fg tables are not dumped via pg_dump
   * autocomplete for CREATE FOREIGN DATA WRAPPER doesn't offer HANDLER
   keyword (probably it isn't your problem)
   * ERROR:  unrecognized objkind: 18 issue
   
   create table omega(a int, b int, c int);
   insert into omega select i, i+1, i+2 from generate_series(1,10000000,3) g(i);
   
   
   postgres=# SELECT * from pg_foreign_server ;
    srvname | srvowner | srvfdw | srvtype | srvversion | srvacl | srvoptions
   ---------+----------+--------+---------+------------+--------+------------
    fake    |    16384 |  16385 |         |            |        |
   (1 row)
   
   postgres=# SELECT * from pg_foreign_data_wrapper ;
    fdwname | fdwowner | fdwvalidator | fdwhandler | fdwacl | fdwoptions
   ---------+----------+--------------+------------+--------+------------
    xx      |    16384 |         3120 |       3121 |        |
   (1 row)
   
   COPY omega to '/tmp/omega';
   
   CREATE FOREIGN TABLE omega3(a int, b int, c int) SERVER fake OPTIONS
   (filename '/tmp/omega');
   
   create role tom;
   grant select on omega2 to tom;
   
   there was unstable behave - first call of select * from omega was
   finished by * ERROR:  unrecognized objkind: 18 (I can't to simulate
   later :( )
   
   second was finished with correct exception
   
   ERROR:  must be superuser to COPY to or from a file
   HINT:  Anyone can COPY to stdout or from stdin. psql's \copy command
   also works for anyone.
   
   Have to be this security limits still ? I understand to this limit for
   COPY statement, but I don't see a sense for foreign table. I agree -
   only superuser can CREATE FOREIGN TABLE based on file fdw handler. But
   why access via MED have to be limited?
   
   I am very happy from implementation of file_fdw_handler. It is proof
   so LIMIT isn't a problem, and I don't understand why it have to be a
   problem for dblink handler.
   
   
   postgres=# select count(*) from omega2;
     count
   ---------
    3335004
   (1 row)
   
   Time: 1915,281 ms
   postgres=# select count(*) from omega2;
     count
   ---------
    3335004
   (1 row)
   
   Time: 1921,744 ms
   
   postgres=# select count(*) from (select * from omega2 limit 1000) x;
    count
   -------
     1000
   (1 row)
   
   Time: 1,597 ms
   
   From practical view I like to see a used option for any tables. I am
   missing a more described info in \d command
   
   Regards
   
   Pavel
   
   
   
   
   2010/7/20 Itagaki Takahiro <itagaki.takahiro@gmail.com>:
   > 2010/7/14 Pavel Stehule <pavel.stehule@gmail.com>:
   >> please, can you refresh patch, please?
   >
   > Updated patch attached. The latest version is always in the git repo.
   > http://repo.or.cz/w/pgsql-fdw.git   (branch: fdw)
   > I'm developing the patch on postgres' git repo. So, regression test
   > for dblink might fail because of out-of-sync issue between cvs and git.
   >
   >> When I looked to documentation I miss a some tutorial for foreign
   >> tables. There are only reference. I miss some paragraph where is
   >> cleanly and simple specified what is possible now and whot isn't
   >> possible. Enhancing of dblink isn't documented
   >
   > Sure. I'll start to write documentation when we agree the design of FDW.
   >
   >> In function  pgIterate(ForeignScanState *scanstate) you are iterare
   >> via pg result. I am thinking so using a cursor and fetching multiple
   >> rows should be preferable.
   >
   > Sure, but I'm thinking that it will be improved after libpq supports
   > protocol-level cursor. The libpq improvement will be applied
   > much more applications including postgresql_fdw.
   >
   > --
   > Itagaki Takahiro
   >