Is this right?
postgres=# \d+ agg_text
Foreign table public.agg_text
Column | Type | Modifiers | Storage | Description
+--+---+--+-
a | smallint | | plain|
b | text | | extended |
Server:
Shigeru HANADA han...@metrosystems.co.jp writes:
[ 20110218-file_fdw.patch ]
I've adjusted this to fit the extensions infrastructure and the
committed version of the FDW API patch, and applied it.
* You might forget some combination or unspecified options in
file_fdw_validator().
For
On Wed, 16 Feb 2011 16:48:33 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Tue, Feb 8, 2011 at 00:30, Shigeru HANADA han...@metrosystems.co.jp
wrote:
Fixed version is attached.
I reviewed your latest git version, that is a bit newer than the attached
patch.
On Tue, Feb 8, 2011 at 00:30, Shigeru HANADA han...@metrosystems.co.jp wrote:
Fixed version is attached.
I reviewed your latest git version, that is a bit newer than the attached patch.
On Sun, Feb 13, 2011 at 12:41:11PM -0600, Kevin Grittner wrote:
Unpatched:
real17m24.171s
real16m52.892s
real16m40.624s
real16m41.700s
Patched:
real15m56.249s
real15m47.001s
real15m3.018s
real17m16.157s
Since you said that a cursory test, or no test
On Mon, Feb 14, 2011 at 22:06, Noah Misch n...@leadboat.com wrote:
On Sun, Feb 13, 2011 at 12:41:11PM -0600, Kevin Grittner wrote:
Unpatched:
real 17m24.171s
real 16m52.892s
real 16m40.624s
real 16m41.700s
Patched:
real 15m56.249s
real 15m47.001s
real 15m3.018s
Noah Misch n...@leadboat.com wrote:
On Sat, Feb 12, 2011 at 03:42:17PM -0600, Kevin Grittner wrote:
Do you see any reason that COPY FROM should be significantly
*faster* with the patch?
No. Up to, say, 0.5% wouldn't be too surprising, but 8.4% is
surprising.
What is the uncertainty of
On 02/12/2011 05:33 PM, Noah Misch wrote:
On Sat, Feb 12, 2011 at 03:42:17PM -0600, Kevin Grittner wrote:
In two hours of testing with a 90GB production database, the copy
patch on top of HEAD ran 0.6% faster than HEAD for pg_dumpall
(generating identical output files), but feeding that in to
Noah Misch n...@leadboat.com wrote:
I'd say, run them with this patch alone. The important thing is
to not penalize existing COPY users. Incidentally, the did you
want ... ? was a genuine question. I see very little performance
risk here, so the tests could be quite cursory, even absent
On Sat, Feb 12, 2011 at 03:42:17PM -0600, Kevin Grittner wrote:
In two hours of testing with a 90GB production database, the copy
patch on top of HEAD ran 0.6% faster than HEAD for pg_dumpall
(generating identical output files), but feeding that in to and
empty cluster with psql ran 8.4%
On Wed, Feb 9, 2011 at 2:03 AM, Noah Misch n...@leadboat.com wrote:
From a functional and code structure perspective, I find this ready to commit.
(I assume you'll drop the XXX: indent only comments on commit.) Kevin, did
you
want to do that performance testing you spoke of?
OK, so is this
On Sat, Feb 12, 2011 at 01:12, Robert Haas robertmh...@gmail.com wrote:
On Wed, Feb 9, 2011 at 2:03 AM, Noah Misch n...@leadboat.com wrote:
From a functional and code structure perspective, I find this ready to
commit.
(I assume you'll drop the XXX: indent only comments on commit.) Kevin,
Robert Haas robertmh...@gmail.com wrote:
Noah Misch n...@leadboat.com wrote:
From a functional and code structure perspective, I find this
ready to commit. (I assume you'll drop the XXX: indent only
comments on commit.) Kevin, did you want to do that performance
testing you spoke of?
OK,
On Fri, Feb 11, 2011 at 11:31 AM, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
On Sat, Feb 12, 2011 at 01:12, Robert Haas robertmh...@gmail.com wrote:
On Wed, Feb 9, 2011 at 2:03 AM, Noah Misch n...@leadboat.com wrote:
From a functional and code structure perspective, I find this ready to
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
Basically, we have no more tasks until the FDW core API is
applied. COPY API and file_fdw patches are waiting for it.
This is something I've found confusing about this patch set, to the
point of not knowing what to test, exactly. The COPY
On Fri, Feb 11, 2011 at 10:31:08AM -0600, Kevin Grittner wrote:
Robert Haas robertmh...@gmail.com wrote:
Noah Misch n...@leadboat.com wrote:
From a functional and code structure perspective, I find this
ready to commit. (I assume you'll drop the XXX: indent only
comments on commit.)
Noah Misch n...@leadboat.com wrote:
I'd say, run them with this patch alone. The important thing is
to not penalize existing COPY users.
Sounds good.
Incidentally, the did you want ... ? was a genuine question. I
see very little performance risk here, so the tests could be quite
On Tue, Feb 08, 2011 at 09:25:29PM +0900, Itagaki Takahiro wrote:
Here is a revised patch, that including jagged csv support.
A new exported function is named as NextCopyFromRawFields.
Seems a bit incongruous to handle the OID column in that function. That part
fits with the other per-column
On Wed, Feb 09, 2011 at 02:55:26PM +0900, Itagaki Takahiro wrote:
On Wed, Feb 9, 2011 at 03:49, Noah Misch n...@leadboat.com wrote:
The code still violates the contract of ExecEvalExpr() by calling it with
CurrentMemoryContext != econtext-ecxt_per_tuple_memory.
I'm not sure whether we have
On Mon, Feb 7, 2011 at 16:01, Shigeru HANADA han...@metrosystems.co.jp wrote:
This patch is based on latest FDW API patches which are posted in
another thread SQL/MED FDW API, and copy_export-20110104.patch which
was posted by Itagaki-san.
I have questions about estimate_costs().
* What value
On Mon, Feb 07, 2011 at 03:39:39PM +0900, Itagaki Takahiro wrote:
On Sun, Feb 6, 2011 at 09:01, Noah Misch n...@leadboat.com wrote:
Most parse analysis-type bits of DoCopy() move to BeginCopy().
It would be possible to move more FROM-only or TO-only members in BeginCopy()
into their
On Mon, 7 Feb 2011 21:00:53 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Mon, Feb 7, 2011 at 16:01, Shigeru HANADA han...@metrosystems.co.jp
wrote:
This patch is based on latest FDW API patches which are posted in
another thread SQL/MED FDW API, and copy_export-20110104.patch
On 02/07/2011 01:39 AM, Itagaki Takahiro wrote:
file_fdw uses CopyFromErrorCallback() to give errors the proper context. The
function uses template strings like COPY %s, line %d, where %s is the name of
the relation being copied. Presumably file_fdw and other features using this
API would
On Fri, 4 Feb 2011 10:10:56 -0500
Robert Haas robertmh...@gmail.com wrote:
Was there supposed to be a patch attached here? Or where is it? We
are past out of time to get this committed, and there hasn't been a
new version in more than two weeks.
Sorry for late to post patches.
Attached are
On Mon, Jan 17, 2011 at 06:33:08PM -0600, Kevin Grittner wrote:
Itagaki Takahiro wrote:
Shigeru HANADA wrote:
Attached patch would avoid this leak by adding per-copy context to
CopyState. This would be overkill, and ResetCopyFrom() might be
reasonable though.
Good catch. I merged
On Fri, Jan 21, 2011 at 8:12 AM, Shigeru HANADA
han...@metrosystems.co.jp wrote:
* Try strVal() instead of DefElem-val.str
* FdwEPrivate seems too abbreviated for me. How about FileFdwPrivate?
Thanks, fixed.
Was there supposed to be a patch attached here? Or where is it? We
are past out of
On Tue, Jan 18, 2011 at 09:33, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
Review for CF:
Thank your for the review!
Since it doesn't appear to be intended to change any user-visible
behavior, I don't see any need for docs or changes to the regression
tests.
There might be some
On Thu, 20 Jan 2011 22:21:37 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Wed, Jan 19, 2011 at 00:34, Shigeru HANADA han...@metrosystems.co.jp
wrote:
Attached patch requires FDW API patches and copy_export-20110114.patch.
Some minor comments:
Thanks for the comments.
I'll
On Fri, Jan 21, 2011 at 22:12, Shigeru HANADA han...@metrosystems.co.jp wrote:
My concern is the explainInfo interface is not ideal for the purpose
and therefore it will be unstable interface. If we support nested plans
in FDWs, each FDW should receive a tree writer used internally in
On Fri, Jan 21, 2011 at 8:59 AM, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
On Fri, Jan 21, 2011 at 22:12, Shigeru HANADA han...@metrosystems.co.jp
wrote:
My concern is the explainInfo interface is not ideal for the purpose
and therefore it will be unstable interface. If we support
On Wed, Jan 19, 2011 at 00:34, Shigeru HANADA han...@metrosystems.co.jp wrote:
Attached patch requires FDW API patches and copy_export-20110114.patch.
Some minor comments:
* Can you pass slot-tts_values and tts_isnull directly to NextCopyFrom()?
It won't allocate the arrays; just fill the array
On Tue, 18 Jan 2011 15:17:12 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Sat, Jan 15, 2011 at 08:35, Shigeru HANADA han...@metrosystems.co.jp
wrote:
Interface of NextCopyFrom() is fixed to return HeapTuple, to support
tableoid without any change to TupleTableSlot.
3)
Itagaki Takahiro wrote:
Shigeru HANADA wrote:
Attached patch would avoid this leak by adding per-copy context to
CopyState. This would be overkill, and ResetCopyFrom() might be
reasonable though.
Good catch. I merged your fix into the attached patch.
Review for CF:
While there is a
On Sat, Jan 15, 2011 at 08:35, Shigeru HANADA han...@metrosystems.co.jp wrote:
Interface of NextCopyFrom() is fixed to return HeapTuple, to support
tableoid without any change to TupleTableSlot.
3) 20110114-copy_export_HeapTupe.patch
This patch fixes interface of NextCopyFrom() to return
On Fri, 14 Jan 2011 14:20:20 +0900
Shigeru HANADA han...@metrosystems.co.jp wrote:
On Fri, 14 Jan 2011 13:03:27 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
Good catch. I merged your fix into the attached patch.
Thanks, I'll rebase my patches.
I've rebased WIP patches for
On Fri, 7 Jan 2011 10:57:17 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Mon, Dec 20, 2010 at 20:42, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
I added comments and moved some setup codes for COPY TO to BeginCopyTo()
for maintainability. CopyTo() still contains parts
On Fri, 14 Jan 2011 13:03:27 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
Good catch. I merged your fix into the attached patch.
Thanks, I'll rebase my patches.
BTW, why didn't planner choose a materialized plan for the inner loop?
FDW scans are typically slower than heap scans
On Fri, Jan 14, 2011 at 14:20, Shigeru HANADA han...@metrosystems.co.jp wrote:
After copying statisticsof pgbench_xxx tables into csv_xxx tables,
planner generates same plans as for local tables, but costs of
ForeignScan nodes are little lower than them of SeqScan nodes.
Forced Nested Loop
On Mon, 10 Jan 2011 19:26:11 -0500
Tom Lane t...@sss.pgh.pa.us wrote:
Shigeru HANADA han...@metrosystems.co.jp writes:
For the purpose of file_fdw, additional ResetCopyFrom() would be
necessary. I'm planning to include such changes in file_fdw patch.
Please find attached partial patch for
On Fri, 7 Jan 2011 10:57:17 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
I updated the COPY FROM API patch.
- GetCopyExecutorState() is removed because FDWs will use their own context.
The patch just rearranges codes for COPY FROM to export those functions.
It also modifies some
Shigeru HANADA han...@metrosystems.co.jp writes:
For the purpose of file_fdw, additional ResetCopyFrom() would be
necessary. I'm planning to include such changes in file_fdw patch.
Please find attached partial patch for ResetCopyFrom(). Is there
anything else which should be done at reset?
On Fri, 24 Dec 2010 11:09:16 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Tue, Dec 21, 2010 at 21:32, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
On Tue, Dec 21, 2010 at 20:14, Shigeru HANADA han...@metrosystems.co.jp
wrote:
Attached is the revised version of
On Fri, Dec 24, 2010 at 20:04, Shigeru HANADA han...@metrosystems.co.jp wrote:
Iterate is called in query context,
Is it an unavoidable requirement? If possible, I'd like to use per-tuple memory
context as the default. We use per-tuple context in FunctionScan for SETOF
functions. I hope we could
On Tue, Dec 21, 2010 at 21:32, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
On Tue, Dec 21, 2010 at 20:14, Shigeru HANADA han...@metrosystems.co.jp
wrote:
Attached is the revised version of file_fdw patch. This patch is
based on Itagaki-san's copy_export-20101220.diff patch.
#1.
On Mon, 20 Dec 2010 20:42:38 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Sun, Dec 19, 2010 at 12:45, Robert Haas robertmh...@gmail.com wrote:
I'm not questioning any of that. But I'd like the resulting code to
be as maintainable as we can make it.
I added comments and
On Tue, Dec 21, 2010 at 20:14, Shigeru HANADA han...@metrosystems.co.jp wrote:
Attached is the revised version of file_fdw patch. This patch is
based on Itagaki-san's copy_export-20101220.diff patch.
#1. Don't you have per-tuple memory leak? I added GetCopyExecutorState()
because the caller
On Mon, Dec 20, 2010 at 6:42 AM, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
On Sun, Dec 19, 2010 at 12:45, Robert Haas robertmh...@gmail.com wrote:
I'm not questioning any of that. But I'd like the resulting code to
be as maintainable as we can make it.
I added comments and moved
On Fri, Dec 17, 2010 at 11:01 PM, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
On Fri, Dec 17, 2010 at 11:49, Shigeru HANADA han...@metrosystems.co.jp
wrote:
I've just moved permission check and read-only check from BeginCopy()
to DoCopy(). Please see attached patch.
Thanks!
Are
On Sun, Dec 19, 2010 at 12:18, Robert Haas robertmh...@gmail.com wrote:
I'm sort of suspicious of the fact that BeginCopyTo() is a shell
around BeginCopy() while BeginCopyFrom() does a whole bunch of other
stuff. I haven't grokked what the code is doing here well enough to
have a concrete
On Sat, Dec 18, 2010 at 10:43 PM, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
On Sun, Dec 19, 2010 at 12:18, Robert Haas robertmh...@gmail.com wrote:
I'm sort of suspicious of the fact that BeginCopyTo() is a shell
around BeginCopy() while BeginCopyFrom() does a whole bunch of other
On Fri, Dec 17, 2010 at 11:49, Shigeru HANADA han...@metrosystems.co.jp wrote:
I've just moved permission check and read-only check from BeginCopy()
to DoCopy(). Please see attached patch.
Thanks!
Are there any objections for the change? If acceptable,
I'd like to apply it prior to SQL/MED
On Tue, 14 Dec 2010 15:51:18 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Tue, Dec 14, 2010 at 15:31, Shigeru HANADA han...@metrosystems.co.jp
wrote:
In addition to above, ResetCopyFrom() is necessary to support nested
loops which inner node is a ForeignScan.
I think you
On Thu, Dec 16, 2010 at 18:45, Shigeru HANADA han...@metrosystems.co.jp wrote:
COPY FROM is a command which INSERT data from a file essentially,
so it requires RowExclusiveLock on the target table. On the other
hand, file_fdw is a feature which reads data from a file through a
table, so it
On Thu, Dec 16, 2010 at 5:35 AM, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
Ah, I found my bug in BeginCopy(), but it's in the usage of
ExecCheckRTPerms() rather than RowExclusiveLock, right?
The target relation should have been opened and locked by the caller.
I think we can move the
On Thu, Dec 16, 2010 at 23:09, Robert Haas robertmh...@gmail.com wrote:
I believe that our project policy is that permissions checks must be
done at execution time, not parse/plan time.
Oops, yes. I should have said permission checks for foreign tables
should have done in their own execution.
On Thu, 16 Dec 2010 19:35:56 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Thu, Dec 16, 2010 at 18:45, Shigeru HANADA han...@metrosystems.co.jp
wrote:
COPY FROM is a command which INSERT data from a file essentially,
so it requires RowExclusiveLock on the target table. On the
Hi hackers,
Attached is the revised WIP version of file_fdw patch. This patch
should be applied after both of fdw_syntax and fdw_scan patches, which
have been posted to another thread SQL/MED - core functionality.
In this version, file_fdw consists of two parts, file_fdw core part
and copy of
On 12/13/2010 01:31 AM, Itagaki Takahiro wrote:
On Sat, Dec 11, 2010 at 05:30, Andrew Dunstanand...@dunslane.net wrote:
On 12/04/2010 11:11 PM, Itagaki Takahiro wrote:
One exports the copy functions from the core, and another
implements file_fdw using the infrastructure.
Who is actually
Andrew Dunstan and...@dunslane.net writes:
Hmm. I don't think that's going to expose enough for what I want to be
able to do. I actually had in mind exposing lower level routines like
CopyReadAttibutesCSV/CopyReadAttributesText and allowing the Foreign
Data Wrapper to manipulate the raw
On 12/13/2010 11:12 AM, Tom Lane wrote:
Andrew Dunstanand...@dunslane.net writes:
Hmm. I don't think that's going to expose enough for what I want to be
able to do. I actually had in mind exposing lower level routines like
CopyReadAttibutesCSV/CopyReadAttributesText and allowing the Foreign
On Tue, 14 Dec 2010 12:01:36 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:
On Tue, Dec 14, 2010 at 01:25, Andrew Dunstan and...@dunslane.net wrote:
On 12/13/2010 11:12 AM, Tom Lane wrote:
In that case I guess I'll need to do what Shigeru-san has done, and copy
large parts of
On Tue, Dec 14, 2010 at 15:31, Shigeru HANADA han...@metrosystems.co.jp wrote:
- BeginCopyFrom(rel, filename, attnamelist, options) : CopyState
- EndCopyFrom(cstate) : void
- NextCopyFrom(cstate, OUT values, OUT nulls, OUT tupleOid) : bool
- GetCopyExecutorState(cstate) : EState *
-
On Sat, Dec 11, 2010 at 05:30, Andrew Dunstan and...@dunslane.net wrote:
On 12/04/2010 11:11 PM, Itagaki Takahiro wrote:
One exports the copy functions from the core, and another
implements file_fdw using the infrastructure.
Who is actually going to do this split?
I'm working for it :-) I
On 12/04/2010 11:11 PM, Itagaki Takahiro wrote:
On Sun, Dec 5, 2010 at 07:24, Andrew Dunstanand...@dunslane.net wrote:
Looking at file_parser.c, it seems to be largely taken from copy.c. Wouldn't
it be better to call those functions, or refactor them so they are callable
if necessary?
We
2010/11/25 Shigeru HANADA han...@metrosystems.co.jp:
Hi, hackers,
Attached is a patch that adds file_fdw, FDW which reads records from
files on the server side, as a contrib module. This patch is based on
SQL/MED core functionality patch.
[SQL/MED - core functionality]
On Mon, Dec 6, 2010 at 5:48 AM, Hitoshi Harada umi.tan...@gmail.com wrote:
I think it is better to add encoding option to FileFdwOption. In the
patch the encoding of file is assumed as client_encoding, but we may
want to SELECT from different-encoded csv in a query.
Apart from the issue with
On 12/04/2010 11:11 PM, Itagaki Takahiro wrote:
On Sun, Dec 5, 2010 at 07:24, Andrew Dunstanand...@dunslane.net wrote:
Looking at file_parser.c, it seems to be largely taken from copy.c. Wouldn't
it be better to call those functions, or refactor them so they are callable
if necessary?
We
On 11/25/2010 03:12 AM, Shigeru HANADA wrote:
Hi, hackers,
Attached is a patch that adds file_fdw, FDW which reads records from
files on the server side, as a contrib module. This patch is based on
SQL/MED core functionality patch.
[SQL/MED - core functionality]
On Sun, Dec 5, 2010 at 07:24, Andrew Dunstan and...@dunslane.net wrote:
Looking at file_parser.c, it seems to be largely taken from copy.c. Wouldn't
it be better to call those functions, or refactor them so they are callable
if necessary?
We could export private functions and structs in
Hi, hackers,
Attached is a patch that adds file_fdw, FDW which reads records from
files on the server side, as a contrib module. This patch is based on
SQL/MED core functionality patch.
[SQL/MED - core functionality]
http://archives.postgresql.org/pgsql-hackers/2010-11/msg01698.php
File_fdw
On Thu, 25 Nov 2010 17:12:44 +0900
Shigeru HANADA han...@metrosystems.co.jp wrote:
Attached is a patch that adds file_fdw, FDW which reads records from
files on the server side, as a contrib module. This patch is based on
SQL/MED core functionality patch.
[SQL/MED - core functionality]
On Thu, Nov 25, 2010 at 05:51:11PM +0900, Shigeru HANADA wrote:
On Thu, 25 Nov 2010 17:12:44 +0900
Shigeru HANADA han...@metrosystems.co.jp wrote:
Attached is a patch that adds file_fdw, FDW which reads records from
files on the server side, as a contrib module. This patch is based on
On Thu, 25 Nov 2010 18:40:09 -0800
David Fetter da...@fetter.org wrote:
On Thu, Nov 25, 2010 at 05:51:11PM +0900, Shigeru HANADA wrote:
I'm going to add new CommitFest items for this patch and SQL/MED -
postgresql_fdw patch which have been split from SQL/MED patch. Can
I add them to CF
73 matches
Mail list logo
