Re: sql/med review - problems with patching

Hello

please, can you refresh patch, please?

[pavel(at)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

2010/7/14 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

2010/7/20 Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>:
> 2010/7/14 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

On Tue, Jul 20, 2010 at 11:40:18AM +0200, Pavel Stehule wrote:
> 2010/7/20 Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>:
> > 2010/7/14 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)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

2010/7/20 David Fetter <david(at)fetter(dot)org>:
> On Tue, Jul 20, 2010 at 11:40:18AM +0200, Pavel Stehule wrote:
>> 2010/7/20 Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>:
>> > 2010/7/14 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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(at)fetter(dot)org> http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter      XMPP: david(dot)fetter(at)gmail(dot)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
>

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(dot)takahiro(at)gmail(dot)com>:
> 2010/7/14 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>