Tom Lane wrote:
Applied with a moderate amount of editorialization.
Thank you!
Notably, I didn't
like what you'd done with the EvalPlanQual stuff, and after a bit of
reflection decided that the right thing was to teach EvalPlanQual to
execute just the desired subplan.
I didn't really like t
Marko Tiikkaja writes:
> Tom Lane wrote:
>> Could you pull out a patch that includes those changes, please?
> Sorry for the delay, my master was a bit behind :-( . I moved the
> trigger code to nodeDml.c with minor changes and removed unused
> resultRelation stuff from DML nodes completely. Thi
Marko Tiikkaja writes:
> Tom Lane wrote:
>> Hm, I've not compared the two versions of the patch, but what I was
>> thinking was that I'd like to get the resultRelation stuff out of EState
>> and have it *only* in the DML nodes. It sounds like you went in the
>> other direction --- what was the re
Tom Lane wrote:
Hm, I've not compared the two versions of the patch, but what I was
thinking was that I'd like to get the resultRelation stuff out of EState
and have it *only* in the DML nodes. It sounds like you went in the
other direction --- what was the reason for that?
I've taken a compro
Marko Tiikkaja writes:
> Sorry for the delay, my master was a bit behind :-( . I moved the
> trigger code to nodeDml.c with minor changes and removed unused
> resultRelation stuff from DML nodes completely. This also has the
> README stuff in it.
Hm, I've not compared the two versions of the pa
Marko Tiikkaja writes:
> I'm sorry, it didn't occur to me that this is part of this patch, but I
> made these changes in my writeable CTE repo, see
> http://git.postgresql.org/gitweb?p=writeable_cte.git;a=commitdiff;h=41ad3d24af845c67a3866e0946129cf9809fe7e9
Could you pull out a patch that includ
Tom Lane wrote:
I was going to look at it and probably do it myself unless it turns out
to be a bigger change than I'm thinking. The only other stuff that's
Ready for Committer for me now is privileges stuff, which is not on the
critical path for other development work, and besides I'm a bit tir
On Thu, Oct 8, 2009 at 4:43 PM, Tom Lane wrote:
> Robert Haas writes:
>> That seems like a good design. I hate to have you spend too much time
>> on any one patch at this stage of the game, though. Is this a big
>> enough change that we should send it back for the patch author (or
>> other inte
Robert Haas writes:
> That seems like a good design. I hate to have you spend too much time
> on any one patch at this stage of the game, though. Is this a big
> enough change that we should send it back for the patch author (or
> other interested parties) to do make that change, or are you just
On Thu, Oct 8, 2009 at 4:01 PM, Tom Lane wrote:
> Robert Haas writes:
>> On Thu, Oct 8, 2009 at 3:30 PM, Tom Lane wrote:
>>> While we're discussing explain output ... what about triggers?
>>> An important aspect of this change is that at least the row-level
>>> triggers are now going to be charg
On Thu, Oct 8, 2009 at 3:53 PM, Tom Lane wrote:
> Alvaro Herrera writes:
>> Robert Haas escribió:
>>> On Thu, Oct 8, 2009 at 3:30 PM, Tom Lane wrote:
I notice also that the patch has chosen to represent Dml in XML/JSON
explain output as Node Type = Dml with an entirely new attribute
>>
Robert Haas writes:
> On Thu, Oct 8, 2009 at 3:30 PM, Tom Lane wrote:
>> While we're discussing explain output ... what about triggers?
>> An important aspect of this change is that at least the row-level
>> triggers are now going to be charged as runtime of the
>> Dml-or-whatever-we-call-it node
Alvaro Herrera writes:
> Robert Haas escribió:
>> On Thu, Oct 8, 2009 at 3:30 PM, Tom Lane wrote:
>>> I notice also that the patch has chosen to represent Dml in XML/JSON
>>> explain output as Node Type = Dml with an entirely new attribute
>>> Operation to indicate Insert/Update/Delete. Do we re
Robert Haas escribió:
> On Thu, Oct 8, 2009 at 3:30 PM, Tom Lane wrote:
> > I notice also that the patch has chosen to represent Dml in XML/JSON
> > explain output as Node Type = Dml with an entirely new attribute
> > Operation to indicate Insert/Update/Delete. Do we really want to
> > go there?
On Thu, Oct 8, 2009 at 3:30 PM, Tom Lane wrote:
> Alvaro Herrera writes:
>> Tom Lane escribió:
>> Does it affect how is it going to look in EXPLAIN output?
>
> Hmm, I suppose there's not any law that says EXPLAIN has to print the
> same name we use internally. The explain output could say "Inser
Alvaro Herrera writes:
> Tom Lane escribió:
> Does it affect how is it going to look in EXPLAIN output?
Hmm, I suppose there's not any law that says EXPLAIN has to print the
same name we use internally. The explain output could say "Insert",
"Update", or "Delete" independently of what we call th
Tom Lane wrote:
Does anyone have a problem with the ModifyTable suggestion,
or a better idea?
Lacking a better idea, +1 for ModifyTable from me.
Regards,
Marko Tiikkaja
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.post
Tom Lane escribió:
> I wrote:
> > Alvaro Herrera writes:
> >> BTW what was the conclusion of the idea about having three separate
> >> nodes Insert, Delete, Update instead of a single Dml node?
>
> > If we stick with a single node type then I'd suggest calling it
> > something like ModifyTable.
>
I wrote:
> Alvaro Herrera writes:
>> BTW what was the conclusion of the idea about having three separate
>> nodes Insert, Delete, Update instead of a single Dml node?
> If we stick with a single node type then I'd suggest calling it
> something like ModifyTable.
I'm starting to work on this patc
Tom Lane wrote:
Yeah, rewrite rules are going to be a *serious* stumbling block to
this whole concept. Maybe we should just punt the project until we
have a clear idea of how to do that.
I've implemented rewrite rules for writeable CTEs, and at least now I
don't see any problems except one. I
On Oct 4, 2009, at 1:24 PM, David Fetter wrote:
On Sun, Oct 04, 2009 at 01:16:50PM -0400, Tom Lane wrote:
Marko Tiikkaja writes:
If I understood you correctly, this would imply that you wouldn't
be able to do for example:
INSERT INTO foo
WITH t AS ( DELETE FROM bar RETURNING * )
SELECT *
On Sun, Oct 04, 2009 at 01:16:50PM -0400, Tom Lane wrote:
> Marko Tiikkaja writes:
> > If I understood you correctly, this would imply that you wouldn't
> > be able to do for example:
>
> > INSERT INTO foo
> > WITH t AS ( DELETE FROM bar RETURNING * )
> > SELECT * FROM t;
>
> Um ... forget what
Marko Tiikkaja writes:
> If I understood you correctly, this would imply that you wouldn't be
> able to do for example:
> INSERT INTO foo
> WITH t AS ( DELETE FROM bar RETURNING * )
> SELECT * FROM t;
Um ... forget what I said --- not enough caffeine yet, apparently.
Yeah, rewrite rules are goi
On Oct 4, 2009, at 11:47 AM, Tom Lane wrote:
Robert Haas writes:
Well, I think a patch to implement writeable CTEs is probably going
to
have to handle this case - I don't think we can just ignore rewrite
rules when processing a CTE. But it does seem to be beyond the scope
of the current pa
Tom Lane wrote:
Any given executor run only
has to think about one type of DML command --- otherwise the executor
would be broken already, since it takes only one command-type argument.
If I understood you correctly, this would imply that you wouldn't be
able to do for example:
INSERT INTO foo
Robert Haas writes:
> Well, I think a patch to implement writeable CTEs is probably going to
> have to handle this case - I don't think we can just ignore rewrite
> rules when processing a CTE. But it does seem to be beyond the scope
> of the current patch.
I hadn't been paying too much attentio
Robert Haas wrote:
I'm not saying that we don't
want to provide the means to do this, but writeable CTEs alone aren't
meant to handle this.
Well, I think a patch to implement writeable CTEs is probably going to
have to handle this case - I don't think we can just ignore rewrite
rules when proce
On Sun, Oct 4, 2009 at 8:14 AM, Robert Haas wrote:
> Marko -
>
> I noticed something a little odd about the new append-plan handling.
>
> rhaas=# explain update parent set c = 1;
> QUERY PLAN
> ---
>
Marko -
I noticed something a little odd about the new append-plan handling.
rhaas=# explain update parent set c = 1;
QUERY PLAN
---
Update (cost=0.00..60.80 rows=4080 width=12)
-> Seq Scan on
On Fri, Oct 2, 2009 at 7:37 AM, Marko Tiikkaja
wrote:
> Robert Haas wrote:
>>
>> Now the point here is that I eventually want to be able to write
>> something like this:
>>
>> with foo as (insert into project (name) values ('Writeable CTEs')
>> returning id) select * from foo;
>>
>> ...but how doe
Robert Haas wrote:
Now the point here is that I eventually want to be able to write
something like this:
with foo as (insert into project (name) values ('Writeable CTEs')
returning id) select * from foo;
...but how does this get me any closer? It seems to me that the plan
for THAT statement ha
Robert Haas wrote:
Thanks. I read through this patch some more tonight and I guess I am
a bit confused about what it accomplishes. AIUI, the point here is to
lay the groundwork for a future patch to allow writeable CTEs, and I
guess I'm not understanding how it's going to do that.
The purpose
On Thu, Oct 01, 2009 at 10:48:41PM -0400, Robert Haas wrote:
> On Mon, Sep 28, 2009 at 3:19 PM, Marko Tiikkaja
> wrote:
> > Robert Haas wrote:
> >>
> >> Can you at least take a stab at it? We can fix your grammar, but
> >> guessing what's going on without documentation is hard.
> >
> > With some
On Mon, Sep 28, 2009 at 3:19 PM, Marko Tiikkaja
wrote:
> Robert Haas wrote:
>>
>> Can you at least take a stab at it? We can fix your grammar, but
>> guessing what's going on without documentation is hard.
>
> With some help from David Fetter, I took another try at it. I hope
> someone finds thi
Robert Haas writes:
> On Tue, Sep 29, 2009 at 11:29 AM, Alvaro Herrera
> wrote:
>> BTW what was the conclusion of the idea about having three separate
>> nodes Insert, Delete, Update instead of a single Dml node?
> It wasn't obvious from reading the patch why multiple node types would
> be super
On Tue, Sep 29, 2009 at 11:29 AM, Alvaro Herrera
wrote:
> Marko Tiikkaja escribió:
>> Robert Haas wrote:
>> >So I think we should at a minimum ask the patch author to (1) fix the
>> >explain bugs I found and (2) update the README, as well as (3) revert
>> >needless whitespace changes - there are a
Alvaro Herrera writes:
> BTW what was the conclusion of the idea about having three separate
> nodes Insert, Delete, Update instead of a single Dml node?
If we stick with a single node type then I'd suggest calling it
something like ModifyTable.
regards, tom lane
--
Sen
Marko Tiikkaja escribió:
> Robert Haas wrote:
> >So I think we should at a minimum ask the patch author to (1) fix the
> >explain bugs I found and (2) update the README, as well as (3) revert
> >needless whitespace changes - there are a couple in execMain.c, from
> >the looks of it.
>
> In the att
Robert Haas wrote:
Can you at least take a stab at it? We can fix your grammar, but
guessing what's going on without documentation is hard.
With some help from David Fetter, I took another try at it. I hope
someone finds this helpful. I'm happy to answer any questions.
Regards,
Marko Tiikka
On Mon, Sep 28, 2009 at 11:31 AM, Marko Tiikkaja
wrote:
> Robert Haas wrote:
>>
>> So I think we should at a minimum ask the patch author to (1) fix the
>> explain bugs I found and (2) update the README, as well as (3) revert
>> needless whitespace changes - there are a couple in execMain.c, from
Robert Haas writes:
> Heh. I was actually asking an even stupider question, which is why do
> we need to keep all of them in ANY centrally known data structure?
> What operation do we perform that requires us to find all of the
> exstant TTS?
ExecDropTupleTable is used to release slot-related bu
On Sun, Sep 27, 2009 at 12:40 AM, Tom Lane wrote:
> Robert Haas writes:
>> Well, part of the problem is that I've not had a lot of luck trying to
>> understand how the executor really works (what's a tuple table slot
>> and why do we need to know in advance how many of them there are?).
>
> You k
Robert Haas writes:
> Well, part of the problem is that I've not had a lot of luck trying to
> understand how the executor really works (what's a tuple table slot
> and why do we need to know in advance how many of them there are?).
You know, that's actually a really good question. There is not,
Robert Haas wrote:
However, that's not the whole problem, either. To your point about
documentation, it seems this path doesn't touch the README at all, and
it needs to, because some of the statements in that file would
certainly become false were this patch to be applied.
So I think we should
On Tue, Sep 22, 2009 at 11:51 AM, Tom Lane wrote:
> Robert Haas writes:
>> Right now, it looks like most of the code is being shared between all
>> three plan types. I'm pretty suspicious of how much code this patch
>> moves around and how little of it is actually changed. I can't really
>> tel
Robert Haas writes:
> Right now, it looks like most of the code is being shared between all
> three plan types. I'm pretty suspicious of how much code this patch
> moves around and how little of it is actually changed. I can't really
> tell if there's an actual design improvement here or if this
On Tue, Sep 22, 2009 at 11:04 AM, Tom Lane wrote:
> Marko Tiikkaja writes:
>> Robert Haas wrote:
>>> Can you explain the motivation for changing the Append stuff as part
>>> of this patch? It's not immediately clear to me why that needs to be
>>> done as part of this patch or what we get out of
Marko Tiikkaja writes:
> Robert Haas wrote:
>> Can you explain the motivation for changing the Append stuff as part
>> of this patch? It's not immediately clear to me why that needs to be
>> done as part of this patch or what we get out of it.
> It seemed to me that the Append on top was only a
(Sorry, forgot to CC hackers)
Robert Haas wrote:
With regard to the changes in explain.c, I think that the way you've
capitalized INSERT, UPDATE, and DELETE is not consistent with our
usual style for labelling nodes. Also, you've failed to set sname, so
this reads from uninitialized memory when
On Sun, Sep 6, 2009 at 6:10 AM, Marko Tiikkaja
wrote:
> Fixed a couple of bugs and renovated ExecInitDml() a bit. Patch attached.
Hi, I'm reviewing this patch for this CommitFest.
With regard to the changes in explain.c, I think that the way you've
capitalized INSERT, UPDATE, and DELETE is not
On 7/31/2009, "Marko Tiikkaja" wrote:
> ..
I seem to be having problems with my email client. The patch should be
attached this time. Sorry for the noise.
Regards,
Marko Tiikkaja
patch3
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make cha
On 7/19/2009, "Tom Lane" wrote:
> The way that I think this should be approached is
> (1) a code-refactoring patch that moves INSERT/UPDATE/DELETE control
> into plan nodes; then
> (2) a feature patch that makes use of that to expose RETURNING in CTEs.
I've been working on this and here's a patc
On Sun, Jul 19, 2009 at 12:39 PM, Tom Lane wrote:
> Robert Haas writes:
>> I think this is a great feature, and it would be REALLY great if it
>> supported UPDATE and DELETE as well.
>
> It won't get applied until it does, and I imagine the patch author
> wasn't expecting any differently. The sub
Robert Haas writes:
> I think this is a great feature, and it would be REALLY great if it
> supported UPDATE and DELETE as well.
It won't get applied until it does, and I imagine the patch author
wasn't expecting any differently. The submission was clearly marked
"WIP" not "ready to apply".
> H
Jaime Casanova wrote:
- why you need a node InsertReturning (see nodeInsertReturning.c) at all?
I couldn't come up with a better way to do this.
and it crashes for triggers (example using regression's int4_tbl)
Right. I never tested this with triggers. The trigger tuple slot isn't
al
On Sat, Jul 18, 2009 at 5:21 PM, Jaime
Casanova wrote:
> On Tue, Jul 7, 2009 at 3:31 PM, Marko Tiikkaja
> wrote:
>> [...] rules and default values don't work.
>> Recursive CTEs don't work either.
[...]
> and it crashes for triggers
I think this is a great feature, and it would be REALLY great if
Jaime Casanova writes:
> On Sat, Jul 18, 2009 at 6:25 PM, Merlin Moncure wrote:
>> Being able to use 'returning' in a subquery is probably the #1 most
>> requested feature for postgresql (it's also a todo). Solving it for
>> 'with' queries is a nice step in the right direction, and sidesteps
>> so
On Sat, Jul 18, 2009 at 6:25 PM, Merlin Moncure wrote:
> On Sat, Jul 18, 2009 at 5:21 PM, Jaime
> Casanova wrote:
>> my questions first:
>> - what's the use case for this?
>
> Being able to use 'returning' in a subquery is probably the #1 most
> requested feature for postgresql (it's also a todo).
On Sat, Jul 18, 2009 at 5:21 PM, Jaime
Casanova wrote:
> my questions first:
> - what's the use case for this?
Being able to use 'returning' in a subquery is probably the #1 most
requested feature for postgresql (it's also a todo). Solving it for
'with' queries is a nice step in the right directi
On Tue, Jul 7, 2009 at 3:31 PM, Marko
Tiikkaja wrote:
> Hello.
>
> Here's a patch(WIP) that implements INSERT .. RETURNING inside a CTE. Should
> apply cleanly against CVS head.
>
> The INSERT query isn't rewritten so rules and default values don't work.
> Recursive CTEs don't work either.
>
my qu
On Fri, Jul 17, 2009 at 10:42:02AM +0300, Peter Eisentraut wrote:
> On Tuesday 07 July 2009 23:31:54 Marko Tiikkaja wrote:
> > Here's a patch(WIP) that implements INSERT .. RETURNING inside a CTE.
>
> Could you supply some test cases to illustrate what this patch accomplishes?
postgres:54321=# CR
On Tuesday 07 July 2009 23:31:54 Marko Tiikkaja wrote:
> Here's a patch(WIP) that implements INSERT .. RETURNING inside a CTE.
Could you supply some test cases to illustrate what this patch accomplishes?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to yo
Hello.
Here's a patch(WIP) that implements INSERT .. RETURNING inside a CTE.
Should apply cleanly against CVS head.
The INSERT query isn't rewritten so rules and default values don't work.
Recursive CTEs don't work either.
Regards,
Marko Tiikkaja
*** a/src/backend/commands/explain.c
--- b/s
63 matches
Mail list logo
