Re: SQL/MED estimated time of arrival?

From: Shigeru HANADA <hanada(at)metrosystems(dot)co(dot)jp>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, Eric Davies <eric(at)barrodale(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: SQL/MED estimated time of arrival?
Date: 2010-11-19 14:55:15
Message-ID: 20101119235514.4107.6989961C@metrosystems.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the comments.

Attached patch includes fixes your comments marked (*), and other
small fixes such as oid system column support by postgresql_fdw and
file_fdw.

On Fri, 19 Nov 2010 12:16:00 +0200
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> ReleaseConnection is a very generic name for a global function, would be
> good to prefix it with "pgsqlfdw" or something. Same with any other
> globally visible functions.

(*)Agreed, merged two files and make all functions other than
postgresql_fdw_handler() private. Refactored name of public functions
defined for file_fdw to have common prefix "FileState".

> Please use the built-in contain_mutable_functions(Node *) instead of
> custom is_immutable_func(). Or at least func_volatile(Oid)

(*)I didn't know the function, thanks. Replaced custom
is_immutable_func() with contain_mutable_functions().

> Is it really a good idea to allow LOCK TABLE on foreign tables in its
> current form? It only locks the local foreign table object, not the
> table in the remote server.

The first reason to allow LOCK TABLE is to make pg_dump be able to
export definition of foreign tables, and second is allow to lock
normal table which has been inherited by foreign table(s). It would
be able to allow FDWs to delegate lock requestto remote server with
new hook in LockTableRecurse() or somewhere, but IMHO locking remote
would be overkill because SQL/MED doesn't mention about lock.

> Sorry if this was fiercely discussed already, but I don't think the file
> FDW belongs in core. I'd rather see it as a contrib module

It's in core from some passive reasons:

- The file_fdw shared codes with COPY FROM heavily in first proposal.
- Built-in FDW makes creating regression tests easier. Especially
CREATE FOREIGN TABLE DDL requires FDW with valid handler.

Moving to contrib would need adding "dummy FDW" or something which has
valid handler to keep regression tests about DDL in core...

> I would've expected the contrib install script to create the foreign
> data wrapper for me. While you can specify options to a foreign data
> wrapper, the CREATE FOREIGN DATA WRAPPER seems similar to CREATE
> LANGUAGE, ie. something that happens when the foreign data wrapper
> library is installed.

(*)It seems to have been deleted by mistake, fixed.

> How do you specify a foreign table that has a different name in the
> remote server? For example, if I wanted to create a foreign table called
> "foo", that fetched rows from a remote table called "bar"?

You can specify name of schema, table and column with generic option
of postgresql_fdw objects.

object | option name | context
--------+-------------+-------------------------
schema | nspname | foreign table
table | relname | foreign table
column | colname | column of foreign table

> I would really like to see the SQL query that's shipped to the remote
> host in EXPLAIN. That's essential information for analyzing a query that
> involves a foreign table.

Me too :-)
You can see the SQL if you set client_min_messages to debug1 or lower,
but it's just debug message.

New hook in ExplainNode, or new attribute of ForeignScan, would be
necessary to show FWD-specific information in EXPLAIN result. ISTM
this issue should be considered with the following issue about
planner-hook because such information should be generated in planner
phase.

> What about transactions? Does the SQL/MED standard have something to say
> about that?

SQL/MED says nothing about transaction management. It's perhaps
because SQL/MED allows only read access.

> In general, I'm surprised that there's no hook at all into the planning
> phase. You have this TODO comment postgresql_fdw:
>
> > /*
> > * TODO: omit (deparse to "NULL") columns which are not used in the
> > * original SQL.
> > *
> > * We must parse nodes parents of this ForeignScan node to determine unused
> > * columns because some columns may be used only in parent Sort/Agg/Limit
> > * nodes.
> > */
>
> Parsing the parents of the ForeignScan node seems like a backwards way
> of solving the problem. The planner should tell the FDW what columns it
> needs. And there should be some way for the FDW to tell the planner
> which quals it can handle, so that the executor doesn't need to recheck
> them.
>
> You could make the planner interface infinitely complicated, but that's
> no excuse for doing nothing at all. The interface needs some thought...

Optimization about column would be minor issue, rather might be
removable. I'll research about foreign path/plan creation again to
figure the issues about planning out.

Regards,
--
Shigeru Hanada

Attachment Content-Type Size
fdw_select_simple_20101119.patch.gz application/octet-stream 103.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Aidan Van Dyk 2010-11-19 14:58:39 Re: Latches with weak memory ordering (Re: max_wal_senders must die)
Previous Message Robert Haas 2010-11-19 14:54:58 Re: Latches with weak memory ordering (Re: max_wal_senders must die)