Thom Brown writes:
> Is this right?
> postgres=# \d+ agg_text
> Foreign table "public.agg_text"
> Column | Type | Modifiers | Storage | Description
> +--+---+--+-
> a | smallint | | plain|
> b | text |
Is this right?
postgres=# \d+ agg_text
Foreign table "public.agg_text"
Column | Type | Modifiers | Storage | Description
+--+---+--+-
a | smallint | | plain|
b | text | | extended |
Server: file_s
Shigeru HANADA 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 example, format == NULL or
On Wed, 16 Feb 2011 16:48:33 +0900
Itagaki Takahiro wrote:
> On Tue, Feb 8, 2011 at 00:30, Shigeru HANADA
> wrote:
> > Fixed version is attached.
>
> I reviewed your latest git version, that is a bit newer than the attached
> patch.
> http://git.postgresql.org/gitweb?p=users/hanada/postgres.
On Tue, Feb 8, 2011 at 00:30, Shigeru HANADA wrote:
> Fixed version is attached.
I reviewed your latest git version, that is a bit newer than the attached patch.
http://git.postgresql.org/gitweb?p=users/hanada/postgres.git;a=commit;h=0e1a1e1b0e168cb3d8ff4d637747d0ba8f7b8d55
The code still works
On Mon, Feb 14, 2011 at 22:06, Noah Misch 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
>
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
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 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 that fig
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% fa
Noah Misch 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
> entirely.
Noah Misch 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
> cursory, even ab
On Fri, Feb 11, 2011 at 10:31:08AM -0600, Kevin Grittner wrote:
> Robert Haas wrote:
> > Noah Misch 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
Itagaki Takahiro 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 API patch and
the patch-on
On Fri, Feb 11, 2011 at 11:31 AM, Itagaki Takahiro
wrote:
> On Sat, Feb 12, 2011 at 01:12, Robert Haas wrote:
>> On Wed, Feb 9, 2011 at 2:03 AM, Noah Misch wrote:
>>> From a functional and code structure perspective, I find this ready to
>>> commit.
>>> (I assume you'll drop the XXX: indent onl
Robert Haas wrote:
> Noah Misch 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 Ready for Committ
On Sat, Feb 12, 2011 at 01:12, Robert Haas wrote:
> On Wed, Feb 9, 2011 at 2:03 AM, Noah Misch 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 per
On Wed, Feb 9, 2011 at 2:03 AM, Noah Misch 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 Ready for Comm
On Wed, Feb 09, 2011 at 02:55:26PM +0900, Itagaki Takahiro wrote:
> On Wed, Feb 9, 2011 at 03:49, Noah Misch 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 such cont
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 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 woul
On Mon, 7 Feb 2011 21:00:53 +0900
Itagaki Takahiro wrote:
> On Mon, Feb 7, 2011 at 16:01, Shigeru HANADA
> 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
On Mon, Feb 07, 2011 at 03:39:39PM +0900, Itagaki Takahiro wrote:
> On Sun, Feb 6, 2011 at 09:01, Noah Misch 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 BeginCopyFrom/To(
On Mon, Feb 7, 2011 at 16:01, Shigeru HANADA 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 does baserel->tuples
On Fri, 4 Feb 2011 10:10:56 -0500
Robert Haas 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 revised version of fi
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
On Fri, Jan 21, 2011 at 8:12 AM, Shigeru HANADA
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 time to get this co
On Fri, Jan 21, 2011 at 8:59 AM, Itagaki Takahiro
wrote:
> On Fri, Jan 21, 2011 at 22:12, Shigeru HANADA
> 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 rece
On Fri, Jan 21, 2011 at 22:12, Shigeru HANADA 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
>> explain.c. explainInfo, th
On Thu, 20 Jan 2011 22:21:37 +0900
Itagaki Takahiro wrote:
> On Wed, Jan 19, 2011 at 00:34, Shigeru HANADA
> wrote:
> > Attached patch requires FDW API patches and copy_export-20110114.patch.
>
> Some minor comments:
Thanks for the comments.
I'll post revised version of patches in new threads.
On Tue, Jan 18, 2011 at 09:33, Kevin Grittner
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 user-visible behaviors in erro
On Wed, Jan 19, 2011 at 00:34, Shigeru HANADA 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 buffers.
* You can pas
On Tue, 18 Jan 2011 15:17:12 +0900
Itagaki Takahiro wrote:
> On Sat, Jan 15, 2011 at 08:35, Shigeru HANADA
> wrote:
> > Interface of NextCopyFrom() is fixed to return HeapTuple, to support
> > tableoid without any change to TupleTableSlot.
> >
> > 3) 20110114-copy_export_HeapTupe.patch
> > This
On Sat, Jan 15, 2011 at 08:35, Shigeru HANADA 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 results as
> HeapTuple.
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 ther
On Fri, 14 Jan 2011 14:20:20 +0900
Shigeru HANADA wrote:
> On Fri, 14 Jan 2011 13:03:27 +0900
> Itagaki Takahiro wrote:
>
> > Good catch. I merged your fix into the attached patch.
>
> Thanks, I'll rebase my patches.
I've rebased WIP patches for file_fdw onto Itagaki-san's recent
copy_export p
On Fri, Jan 14, 2011 at 14:20, Shigeru HANADA 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 uses Materialize node as
On Fri, 14 Jan 2011 13:03:27 +0900
Itagaki Takahiro 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 or TupleTableslot scans,
On Fri, 7 Jan 2011 10:57:17 +0900
Itagaki Takahiro wrote:
> On Mon, Dec 20, 2010 at 20:42, Itagaki Takahiro
> wrote:
> > I added comments and moved some setup codes for COPY TO to BeginCopyTo()
> > for maintainability. CopyTo() still contains parts of initialization,
> > but I've not touched it y
On Mon, 10 Jan 2011 19:26:11 -0500
Tom Lane wrote:
> Shigeru HANADA 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
> > anythin
Shigeru HANADA 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?
Seems like it would be
On Fri, 7 Jan 2011 10:57:17 +0900
Itagaki Takahiro 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 of COPY TO codes inter
On Fri, Dec 24, 2010 at 20:04, Shigeru HANADA 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 have little difference b
On Fri, 24 Dec 2010 11:09:16 +0900
Itagaki Takahiro wrote:
> On Tue, Dec 21, 2010 at 21:32, Itagaki Takahiro
> wrote:
> > On Tue, Dec 21, 2010 at 20:14, Shigeru HANADA
> > wrote:
> >> Attached is the revised version of file_fdw patch. This patch is
> >> based on Itagaki-san's copy_export-20101
On Tue, Dec 21, 2010 at 21:32, Itagaki Takahiro
wrote:
> On Tue, Dec 21, 2010 at 20:14, Shigeru HANADA
> 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 Ge
On Mon, Dec 20, 2010 at 6:42 AM, Itagaki Takahiro
wrote:
> On Sun, Dec 19, 2010 at 12:45, Robert Haas 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 some setup codes for COPY TO to BeginCopyTo(
On Tue, Dec 21, 2010 at 20:14, Shigeru HANADA 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 needs to reset the per-tup
On Mon, 20 Dec 2010 20:42:38 +0900
Itagaki Takahiro wrote:
> On Sun, Dec 19, 2010 at 12:45, Robert Haas 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 some setup codes for COPY TO to BeginCo
On Sat, Dec 18, 2010 at 10:43 PM, Itagaki Takahiro
wrote:
> On Sun, Dec 19, 2010 at 12:18, Robert Haas 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 d
On Sun, Dec 19, 2010 at 12:18, Robert Haas 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 proposal though...
I
On Fri, Dec 17, 2010 at 11:01 PM, Itagaki Takahiro
wrote:
> On Fri, Dec 17, 2010 at 11:49, Shigeru HANADA
> 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 acce
On Fri, Dec 17, 2010 at 11:49, Shigeru HANADA 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 and file_fdw patches.
We
On Thu, 16 Dec 2010 19:35:56 +0900
Itagaki Takahiro wrote:
> On Thu, Dec 16, 2010 at 18:45, Shigeru HANADA
> 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
On Thu, Dec 16, 2010 at 23:09, Robert Haas 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". So, additional chec
On Thu, Dec 16, 2010 at 5:35 AM, Itagaki Takahiro
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 check to DoCopy() as li
On Thu, Dec 16, 2010 at 18:45, Shigeru HANADA 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 requires AccessShareLo
On Tue, 14 Dec 2010 15:51:18 +0900
Itagaki Takahiro wrote:
> On Tue, Dec 14, 2010 at 15:31, Shigeru HANADA
> wrote:
> > In addition to above, ResetCopyFrom() is necessary to support nested
> > loops which inner node is a ForeignScan.
>
> I think you can add ResetCopyFrom() to the core in your n
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 Tue, Dec 14, 2010 at 15:31, Shigeru HANADA wrote:
>> - BeginCopyFrom(rel, filename, attnamelist, options) : CopyState
>> - EndCopyFrom(cstate) : void
>> - NextCopyFrom(cstate, OUT values, OUT nulls, OUT tupleOid) : bool
>> - GetCopyExecutorState(cstate) : EState *
>> - CopyFromErrorCallback(arg
On Tue, 14 Dec 2010 12:01:36 +0900
Itagaki Takahiro wrote:
> On Tue, Dec 14, 2010 at 01:25, Andrew Dunstan 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 copy.c.
>
> I found file_fdw would require
On 12/13/2010 11:12 AM, Tom Lane wrote:
Andrew Dunstan 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 mani
Andrew Dunstan 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 values read (for ex
On 12/13/2010 01:31 AM, Itagaki Takahiro wrote:
On Sat, Dec 11, 2010 at 05:30, Andrew Dunstan 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
On Sat, Dec 11, 2010 at 05:30, Andrew Dunstan 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 extract those
On 12/04/2010 11:11 PM, Itagaki Takahiro wrote:
On Sun, Dec 5, 2010 at 07:24, Andrew Dunstan 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 f
On Mon, Dec 6, 2010 at 5:48 AM, Hitoshi Harada 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 fdw, I've been t
2010/11/25 Shigeru HANADA :
> 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/pgsq
On 12/04/2010 11:11 PM, Itagaki Takahiro wrote:
On Sun, Dec 5, 2010 at 07:24, Andrew Dunstan 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 f
On Sun, Dec 5, 2010 at 07:24, Andrew Dunstan 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 copy.c,
though details
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]
http://archives.postgresql.
On Thu, 25 Nov 2010 18:40:09 -0800
David Fetter 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 2010-11 w
On Thu, Nov 25, 2010 at 05:51:11PM +0900, Shigeru HANADA wrote:
> On Thu, 25 Nov 2010 17:12:44 +0900
> Shigeru HANADA 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 functio
On Thu, 25 Nov 2010 17:12:44 +0900
Shigeru HANADA 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]
> http://archives.postg
73 matches
Mail list logo
