I wrote:
I did a bit of poking around here, and came to the following
conclusions:
1. We don't want to add another RTEKind. RTE_RELATION basically has the
semantics of anything with a pg_class OID, so it ought to include
foreign tables. Therefore the fix ought to be to add relkind to
I wrote:
Robert Haas robertmh...@gmail.com writes:
On Fri, Feb 18, 2011 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote:
The main downside of that is that relation relkinds would have
to become fixed (because there would be no practical way of updating
RTEs in stored rules), which means the
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
On 11.02.2011 22:50, Heikki Linnakangas wrote:
I spent some more time reviewing this, and working on the PostgreSQL FDW
in tandem. Here's an updated API patch, with a bunch of cosmetic
changes, and a bug fix for FOR SHARE/UPDATE.
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
On 15.02.2011 23:00, Tom Lane wrote:
I think moving the error check downstream would be a good thing.
Ok, I tried moving the error checks to preprocess_rowmarks().
Unfortunately RelOptInfos haven't been built at that stage yet,
On Fri, Feb 18, 2011 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote:
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
On 15.02.2011 23:00, Tom Lane wrote:
I think moving the error check downstream would be a good thing.
Ok, I tried moving the error checks to
Robert Haas robertmh...@gmail.com writes:
On Fri, Feb 18, 2011 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote:
The main downside of that is that relation relkinds would have
to become fixed (because there would be no practical way of updating
RTEs in stored rules), which means the convert
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
Another version, rebased against master branch and with a bunch of small
cosmetic fixes.
I guess this is as good as this is going to get for 9.1.
This is *badly* in need of another cleanup pass; it's full of typos,
contradictory
On 18.02.2011 22:16, Tom Lane wrote:
Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes:
Another version, rebased against master branch and with a bunch of small
cosmetic fixes.
I guess this is as good as this is going to get for 9.1.
This is *badly* in need of another cleanup
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
On 18.02.2011 22:16, Tom Lane wrote:
Question after first look: what is the motivation for passing
estate-es_param_list_info to BeginScan? AFAICS, even if there is a
reason for that function to need that, it isn't receiving any
I wrote:
... My feeling is it'd be best to pass down
all the information the executor node has got --- probably we should
just pass the ForeignScanState node itself, and leave a void * in that
for FDW-private data, and be done with it. Otherwise we're going to be
adding missed stuff back to
On 15.02.2011 23:00, Tom Lane wrote:
Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes:
On 15.02.2011 21:13, Tom Lane wrote:
Hmm. I don't have a problem with adding relkind to the planner's
RelOptInfo, but it seems to me that if parse analysis needs to know
this, you have put
On Tue, Feb 15, 2011 at 1:40 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
I'm actually surprised we don't need to distinguish them in more places, but
nevertheless it feels like we should have that info available more
conveniently, and without requiring a catalog lookup
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
As the patch stands, we have to do get_rel_relkind() in a couple of
places in parse analysis and the planner to distinguish a foreign table
from a regular one. As the patch stands, there's nothing in
RangeTblEntry (which is what
On 15.02.2011 21:13, Tom Lane wrote:
Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes:
As the patch stands, we have to do get_rel_relkind() in a couple of
places in parse analysis and the planner to distinguish a foreign table
from a regular one. As the patch stands, there's
On 15.02.2011 21:00, Robert Haas wrote:
On Tue, Feb 15, 2011 at 1:40 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
I'm actually surprised we don't need to distinguish them in more places, but
nevertheless it feels like we should have that info available more
conveniently,
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
On 15.02.2011 21:13, Tom Lane wrote:
Hmm. I don't have a problem with adding relkind to the planner's
RelOptInfo, but it seems to me that if parse analysis needs to know
this, you have put functionality into parse analysis that
On Mon, 07 Feb 2011 09:37:37 +0100
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote:
On 07.02.2011 08:00, Shigeru HANADA wrote:
Sorry for late, attached are revised version of FDW API patches which
reflect Heikki's comments except removing catalog lookup via
IsForeignTable().
On 07.02.2011 08:00, Shigeru HANADA wrote:
Sorry for late, attached are revised version of FDW API patches which
reflect Heikki's comments except removing catalog lookup via
IsForeignTable(). ISTM that the point is avoiding catalog lookup
during planning, but I have not found when we can set
On Mon, 31 Jan 2011 22:00:55 +0900
Shigeru HANADA han...@metrosystems.co.jp wrote:
I'll post FDW API patches which reflect comments first, and then I'll
rebase postgresql_fdw against them.
Sorry for late, attached are revised version of FDW API patches which
reflect Heikki's comments except
On Mon, 24 Jan 2011 15:08:11 +0200
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote:
I've gone through the code in a bit more detail now. I did a bunch of
cosmetic changes along the way, patch attached. I also added a few
paragraphs in the docs. We need more extensive
On Mon, Jan 31, 2011 at 8:00 AM, Shigeru HANADA
han...@metrosystems.co.jp wrote:
* Is there any use case for changing the handler or validator function
of an existign FDW with ALTER? To me it just seems like an unnecessary
complication.
AFAICS, the only case for that is upgrading FDW to new
On Mon, Jan 24, 2011 at 8:08 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
How much review have you done of parts (3) and (4)? The key issue for
all of the FDW work in progress seems to be what the handler API is
going to look like, and so once we get that committed it will
On Fri, 21 Jan 2011 18:28:19 +0200
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote:
On 18.01.2011 17:26, Shigeru HANADA wrote:
3) 20110118-fdw_handler.patch - This patch adds support for HANDLER
option to FOREIGN DATA WRAPPER object.
Some quick comments on that:
Thanks for
On Sat, Jan 22, 2011 at 07:20, Tom Lane t...@sss.pgh.pa.us wrote:
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
* I wonder if CREATE FOREIGN DATA WRAPPER should automatically create
the handler function, if it doesn't exist yet.
Doing that would require the equivalent of
On 21.01.2011 17:57, Robert Haas wrote:
On Fri, Jan 21, 2011 at 10:17 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
On 18.01.2011 17:26, Shigeru HANADA wrote:
1) 20110118-no_fdw_perm_check.patch - This patch is not included in
last post. This had been proposed on
On Mon, Jan 24, 2011 at 8:08 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
* Is there any point in allowing a FDW without a handler? It's totally
useless, isn't it? We had the CREATE FOREIGN DATA WRAPPER syntax in previous
versions, and it allowed it, but it has always been
On 18.01.2011 17:26, Shigeru HANADA wrote:
1) 20110118-no_fdw_perm_check.patch - This patch is not included in
last post. This had been proposed on 2011-01-05 first, but maybe has
not been reviewd yet. I re-propose this patch for SQL standard
conformance. This patch removes permission check
On Fri, Jan 21, 2011 at 10:17 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
On 18.01.2011 17:26, Shigeru HANADA wrote:
1) 20110118-no_fdw_perm_check.patch - This patch is not included in
last post. This had been proposed on 2011-01-05 first, but maybe has
not been reviewd
On 21.01.2011 17:57, Robert Haas wrote:
How much review have you done of parts (3) and (4)?
Not much. I'm getting there..
The key issue for
all of the FDW work in progress seems to be what the handler API is
going to look like, and so once we get that committed it will unblock
a lot of other
On 18.01.2011 17:26, Shigeru HANADA wrote:
3) 20110118-fdw_handler.patch - This patch adds support for HANDLER
option to FOREIGN DATA WRAPPER object.
Some quick comments on that:
* I wonder if CREATE FOREIGN DATA WRAPPER should automatically create
the handler function, if it doesn't exist
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
Some quick comments on that:
* I wonder if CREATE FOREIGN DATA WRAPPER should automatically create
the handler function, if it doesn't exist yet. That's what CREATE
LANGUAGE does, which is similar. Although it doesn't seem to be
On Sun, 16 Jan 2011 01:55:19 +0100
Jan Urbański wulc...@wulczer.org wrote:
snip
In general, the feature looks great and I hope it'll make it into 9.1.
And it we'd get the possibility to write FDW handlers in other PLs than
C, it would rock so hard...
I'm going to mark this a Waiting for
On Sun, 16 Jan 2011 01:55:19 +0100
Jan Urbański wulc...@wulczer.org wrote:
what follows is a review of the FDW API patch from
http://archives.postgresql.org/message-id/20110114212358.82c7.69899...@metrosystems.co.jp
Thanks for the comments!
For now, I answer to the first half of your comments.
On Mon, 17 Jan 2011 22:13:19 +0900
Shigeru HANADA han...@metrosystems.co.jp wrote:
Fixed in attached patch.
Sorry, I have not attached patch to last message.
I'll post revised patch in next message soon.
Regards,
--
Shigeru Hanada
--
Sent via pgsql-hackers mailing list
Hi,
what follows is a review of the FDW API patch from
http://archives.postgresql.org/message-id/20110114212358.82c7.69899...@metrosystems.co.jp
All three patches apply cleanly and compile without warnings. Regression
tests pass.
Let me go patch by patch, starting with the first one that adds
35 matches
Mail list logo
