Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2014-05-15 Thread Arthur Axel 'fREW' Schmidt
Andres Freund anarazel.de> writes: > > Hi, > > Some comments about the patch: > * Coding Style: > * multiline comments have both /* and */ on their own lines. > * I think several places indent by two tabs. > * Spaces around operators > * ... > * Many of the new comments would enjoy a bi

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2014-04-04 Thread Andres Freund
Hi, Some comments about the patch: * Coding Style: * multiline comments have both /* and */ on their own lines. * I think several places indent by two tabs. * Spaces around operators * ... * Many of the new comments would enjoy a bit TLC by a native speaker. * The way RTE_ALIAS creeps in

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2014-02-11 Thread David Fetter
On Sun, Feb 02, 2014 at 02:52:42PM -0800, David Fetter wrote: > On Wed, Aug 21, 2013 at 08:52:25PM +0200, Karol Trzcionka wrote: > > W dniu 21.08.2013 19:17, Boszormenyi Zoltan pisze: > > > With this fixed, a more complete review: > > Thanks. > > I've done some syntactic and white space cleanup, h

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2014-02-02 Thread David Fetter
On Wed, Aug 21, 2013 at 08:52:25PM +0200, Karol Trzcionka wrote: > W dniu 21.08.2013 19:17, Boszormenyi Zoltan pisze: > > With this fixed, a more complete review: > Thanks. I've done some syntactic and white space cleanup, here attached. Karol, would you care to help with commenting the sections

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-11-21 Thread David Fetter
On Fri, Oct 04, 2013 at 10:42:32AM +0200, Karol Trzcionka wrote: > W dniu 04.10.2013 02:51, Robert Haas pisze: > > Do you have a link to previous discussion on the mailing list? > Sorry, most of discussion was at IRC channel. > > I'm not positive there's enough information available > > at that sta

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-10-04 Thread Robert Haas
On Fri, Oct 4, 2013 at 5:04 AM, Marko Tiikkaja wrote: > I might be completely in the woods here, but I believe something like this > was attempted by Karol earlier, and it failed if two concurrent transactions > did something similar to: > > UPDATE foo SET a = a + 1 RETURNING BEFORE.a; > > Both

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-10-04 Thread Robert Haas
On Fri, Oct 4, 2013 at 4:42 AM, Karol Trzcionka wrote: > W dniu 04.10.2013 02:51, Robert Haas pisze: >> Do you have a link to previous discussion on the mailing list? > Sorry, most of discussion was at IRC channel. >> I'm not positive there's enough information available >> at that stage, but if p

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-10-04 Thread Andres Freund
On 2013-10-03 20:51:08 -0400, Robert Haas wrote: > On Thu, Oct 3, 2013 at 7:54 PM, Karol Trzcionka wrote: > > Compare EXPLAIN ANALYZE VERBOSE on your statement and on "patched" > > workflow. I can see significant difference. And your "after" returns the > > value after whole the work (after trigge

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-10-04 Thread Marko Tiikkaja
On 10/4/13 12:28 AM, Robert Haas wrote: In fact, we can already get approximately the desired effect already: rhaas=# update foo as after set a = before.a + 1 from foo as before where before.a = after.a returning before.a, after.a; a | a ---+--- 1 | 2 (1 row) Now this is a hack, because we

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-10-04 Thread Karol Trzcionka
W dniu 04.10.2013 02:51, Robert Haas pisze: > Do you have a link to previous discussion on the mailing list? Sorry, most of discussion was at IRC channel. > I'm not positive there's enough information available > at that stage, but if p_target_rangetblentry is populated at that > point, you should

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-10-03 Thread Robert Haas
On Thu, Oct 3, 2013 at 7:54 PM, Karol Trzcionka wrote: > Compare EXPLAIN ANALYZE VERBOSE on your statement and on "patched" > workflow. I can see significant difference. And your "after" returns the > value after whole the work (after trigger fired) as I know (I don't know > if it is needed or not

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-10-03 Thread Karol Trzcionka
W dniu 04.10.2013 00:28, Robert Haas pisze: > I wonder if we shouldn't be trying to handle resolution of these names > at an earlier processing stage, closer to the processor. Maybe it can be done in parser (in flex?) but at now it seems to be more isolated feature. > In fact, we can already get ap

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-10-03 Thread Robert Haas
I took a look at this patch today, and I'm pretty skeptical about whether it's on the right track. It adds a new kind of RTE called RTE_ALIAS, which doesn't seem particularly descriptive and alias is used elsewhere to mean something fairly different. More generally, I'm not convinced that adding

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-21 Thread Boszormenyi Zoltan
Hi, 2013-08-21 20:52 keltezéssel, Karol Trzcionka írta: W dniu 21.08.2013 19:17, Boszormenyi Zoltan pisze: With this fixed, a more complete review: Thanks. There is a new regression test (returning_before_after.sql) covering this feature. However, I think it should be added to the group where

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-21 Thread Karol Trzcionka
W dniu 21.08.2013 19:17, Boszormenyi Zoltan pisze: > With this fixed, a more complete review: Thanks. > There is a new regression test (returning_before_after.sql) covering > this feature. However, I think it should be added to the group > where "returning.sql" resides currently. There is a value i

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-21 Thread Boszormenyi Zoltan
2013-08-21 20:00 keltezéssel, Boszormenyi Zoltan írta: 2013-08-21 19:17 keltezéssel, Boszormenyi Zoltan írta: Hi, 2013-08-20 21:06 keltezéssel, Karol Trzcionka írta: W dniu 20.08.2013 20:55, Boszormenyi Zoltan pisze: Here's a new one, for v7: setrefs.c: In function ‘set_plan_refs’: setrefs.c

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-21 Thread Boszormenyi Zoltan
2013-08-21 19:17 keltezéssel, Boszormenyi Zoltan írta: Hi, 2013-08-20 21:06 keltezéssel, Karol Trzcionka írta: W dniu 20.08.2013 20:55, Boszormenyi Zoltan pisze: Here's a new one, for v7: setrefs.c: In function ‘set_plan_refs’: setrefs.c:2001:26: warning: ‘before_index’ may be used uninitiali

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-21 Thread Boszormenyi Zoltan
Hi, 2013-08-20 21:06 keltezéssel, Karol Trzcionka írta: W dniu 20.08.2013 20:55, Boszormenyi Zoltan pisze: Here's a new one, for v7: setrefs.c: In function ‘set_plan_refs’: setrefs.c:2001:26: warning: ‘before_index’ may be used uninitialized in this function [-Wmaybe-uninitialized] bind_re

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-20 Thread Karol Trzcionka
W dniu 20.08.2013 20:55, Boszormenyi Zoltan pisze: > Here's a new one, for v7: > > setrefs.c: In function ‘set_plan_refs’: > setrefs.c:2001:26: warning: ‘before_index’ may be used uninitialized > in this function [-Wmaybe-uninitialized] > bind_returning_variables(rlist, before_index, after_index)

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-20 Thread Boszormenyi Zoltan
2013-08-20 17:30 keltezéssel, Karol Trzcionka írta: W dniu 20.08.2013 16:47, Karol Trzcionka pisze: Thank you for the review and tests. New version introduce a lot of improvements: - Fix regression test for view (wrong table_name) - Add regression test for inheritance - Delete hack in initsplan.

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-20 Thread Karol Trzcionka
W dniu 20.08.2013 16:47, Karol Trzcionka pisze: > Thank you for the review and tests. New version introduce a lot of > improvements: > - Fix regression test for view (wrong table_name) > - Add regression test for inheritance > - Delete hack in initsplan.c (now we ignore all RTE_BEFORE) - the > unin

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-20 Thread Karol Trzcionka
Thank you for the review and tests. New version introduce a lot of improvements: - Fix regression test for view (wrong table_name) - Add regression test for inheritance - Delete hack in initsplan.c (now we ignore all RTE_BEFORE) - the uninitialized issue - Revert changing varno in add_vars_to_targe

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-20 Thread Boszormenyi Zoltan
Hi, 2013-08-19 21:52 keltezéssel, Boszormenyi Zoltan írta: 2013-08-19 21:21 keltezéssel, Karol Trzcionka írta: W dniu 19.08.2013 19:56, Boszormenyi Zoltan pisze: * Does it apply cleanly to the current git master? No. There's a reject in src/backend/optimizer/plan/initsplan.c Thank you, merge

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-19 Thread Boszormenyi Zoltan
2013-08-19 21:21 keltezéssel, Karol Trzcionka írta: W dniu 19.08.2013 19:56, Boszormenyi Zoltan pisze: * Does it apply cleanly to the current git master? No. There's a reject in src/backend/optimizer/plan/initsplan.c Thank you, merged in attached version. * Does it include reasonable tests?

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-19 Thread Karol Trzcionka
W dniu 19.08.2013 19:56, Boszormenyi Zoltan pisze: > * Does it apply cleanly to the current git master? > > No. There's a reject in src/backend/optimizer/plan/initsplan.c Thank you, merged in attached version. > > * Does it include reasonable tests? > > Yes but the test fails after trying to fix th

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-08-19 Thread Boszormenyi Zoltan
Hi, mini-review follows. 2013-07-22 21:52 keltezéssel, Karol Trzcionka írta: I've noticed problem with "UPDATE ... FROM" statement. Fix in new version. Regards, Karol * Does it apply cleanly to the current git master? No. There's a reject in src/backend/optimizer/plan/initsplan.c * Does it

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-23 Thread Karol Trzcionka
W dniu 23.07.2013 06:22, David Fetter pisze: > What problem or problems did you notice, and what did you change to > fix them? "UPDATE ... FROM" generated "ERROR: variable not found in subplan target lists". I've added some workaround in add_vars_to_targetlist: - if it is an "after" - simple chang

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-22 Thread David Fetter
On Mon, Jul 22, 2013 at 09:52:14PM +0200, Karol Trzcionka wrote: > I've noticed problem with "UPDATE ... FROM" statement. Fix in new version. > Regards, > Karol What problem or problems did you notice, and what did you change to fix them? Cheers, David. -- David Fetter http://fetter.org/ Phone:

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-22 Thread Karol Trzcionka
I've noticed problem with "UPDATE ... FROM" statement. Fix in new version. Regards, Karol diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml index 90b9208..eba35f0 100644 --- a/doc/src/sgml/ref/update.sgml +++ b/doc/src/sgml/ref/update.sgml @@ -194,12 +194,27 @@ UPDATE [ ONLY

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-19 Thread Karol Trzcionka
New version: - fix returning "after" values if there are not "before" - add more regression tests I'd like to hear/read any feedback ;) Regards, Karol diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml index 90b9208..eba35f0 100644 --- a/doc/src/sgml/ref/update.sgml +++ b/doc/

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-13 Thread David Fetter
On Sat, Jul 13, 2013 at 12:49:45AM +0200, Karol Trzcionka wrote: > Next version: > - cleanup > - regression test > - fix issue reported by johto (invalid values in parallel transactions) > I would like more feedback and comments about the patch, as some parts > may be too hacky. > In particular, is

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-12 Thread Karol Trzcionka
Next version: - cleanup - regression test - fix issue reported by johto (invalid values in parallel transactions) I would like more feedback and comments about the patch, as some parts may be too hacky. In particular, is it a problem that I update a pointer to planSlot? In my patch, it points to tu

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-05 Thread Karol Trzcionka
Updated patch: - include sgml - fix all compiler warnings - some cleanup - fix correctness of feature Regards, Karol diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml index 90b9208..eba35f0 100644 --- a/doc/src/sgml/ref/update.sgml +++ b/doc/src/sgml/ref/update.sgml @@ -194,1

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-05 Thread David Fetter
On Fri, Jul 05, 2013 at 03:22:33AM +0200, Karol Trzcionka wrote: > Hello, > according to my mentor's suggestion, I send first PoC patch of > "RETURNING AFTER/BEFORE" statement. Some info: > - it is early version - more hack PoC than RC patch > - AFTER in this version works as decribed before but it

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-07-04 Thread Karol Trzcionka
Hello, according to my mentor's suggestion, I send first PoC patch of "RETURNING AFTER/BEFORE" statement. Some info: - it is early version - more hack PoC than RC patch - AFTER in this version works as decribed before but it returns row after update but before post-update before (to be fixed or swi

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Tom Lane
Karol Trzcionka writes: > It will not solve the problems: > 1. How to access to old rows if the table is named "BEFORE"? The user can simply choose to use a different table alias, as Andres explained upthread. If any table's active alias is "before" (or "after"), we just don't activate the corre

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Karol Trzcionka
W dniu 02.05.2013 23:34, Gavin Flower pisze: > I prefer 'PRIOR & 'AFTER' as the both have the same length > - but perhaps that is just me! :-) I think it doesn't matter at now because PRIOR has the same problems as BEFORE ;) Regards, Karol

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Gavin Flower
On 03/05/13 04:52, David Fetter wrote: On Thu, May 02, 2013 at 06:28:53PM +0200, Andres Freund wrote: On 2013-05-02 12:23:19 -0400, Tom Lane wrote: Marko Tiikkaja writes: What I'm more interested in is: how can we make this feature work in PL/PgSQL where OLD means something different? That's

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Karol Trzcionka
W dniu 02.05.2013 20:45, Tom Lane pisze: > David Fetter writes: >> Maybe we can make BEFORE and AFTER implied aliases rather than >> keywords. What say? > Well, they still have to be unreserved keywords for their use in > trigger-related commands, but as far as this feature is concerned > I think

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Tom Lane
Karol Trzcionka writes: > What do you think about function- or cast-like syntax. I mean: > RETURNING OLD(foo.bar) > or RETURNING OLD(foo).bar > or RETURNING (foo::OLD).bar ? > I think none of them should conflict with any other statements. I think you'll find those alternatives are at least as ug

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Tom Lane
David Fetter writes: > Maybe we can make BEFORE and AFTER implied aliases rather than > keywords. What say? Well, they still have to be unreserved keywords for their use in trigger-related commands, but as far as this feature is concerned I think they're best off being handled as automatically-s

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Atri Sharma
Sent from my iPad On 03-May-2013, at 0:07, David Fetter wrote: > On Thu, May 02, 2013 at 01:40:59PM -0400, Tom Lane wrote: >> David Fetter writes: >>> On Thu, May 02, 2013 at 06:28:53PM +0200, Andres Freund wrote: prior/after? Both are unreserved keywords atm and it seems far less l

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread David Fetter
On Thu, May 02, 2013 at 01:40:59PM -0400, Tom Lane wrote: > David Fetter writes: > > On Thu, May 02, 2013 at 06:28:53PM +0200, Andres Freund wrote: > >> prior/after? Both are unreserved keywords atm and it seems far less > >> likely to have conflicts than new/old. > > > BEFORE/AFTER seems more lo

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Karol Trzcionka
W dniu 02.05.2013 19:40, Tom Lane pisze: >> BEFORE/AFTER seems more logical to me. > Works for me. > What do you think about function- or cast-like syntax. I mean: RETURNING OLD(foo.bar) or RETURNING OLD(foo).bar or RETURNING (foo::OLD).bar ? I think none of them should conflict with any other stat

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Tom Lane
David Fetter writes: > On Thu, May 02, 2013 at 06:28:53PM +0200, Andres Freund wrote: >> prior/after? Both are unreserved keywords atm and it seems far less >> likely to have conflicts than new/old. > BEFORE/AFTER seems more logical to me. Works for me. regards, tom lane

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Pavel Stehule
2013/5/2 Tom Lane > Marko Tiikkaja writes: > > What I'm more interested in is: how can we make this feature work in > > PL/PgSQL where OLD means something different? > > That's a really good point: if you follow this approach then you're > creating fundamental conflicts for use of the feature in

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread David Fetter
On Thu, May 02, 2013 at 06:28:53PM +0200, Andres Freund wrote: > On 2013-05-02 12:23:19 -0400, Tom Lane wrote: > > Marko Tiikkaja writes: > > > What I'm more interested in is: how can we make this feature work in > > > PL/PgSQL where OLD means something different? > > > > That's a really good po

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Andres Freund
On 2013-05-02 12:23:19 -0400, Tom Lane wrote: > Marko Tiikkaja writes: > > What I'm more interested in is: how can we make this feature work in > > PL/PgSQL where OLD means something different? > > That's a really good point: if you follow this approach then you're > creating fundamental conflic

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Tom Lane
Marko Tiikkaja writes: > What I'm more interested in is: how can we make this feature work in > PL/PgSQL where OLD means something different? That's a really good point: if you follow this approach then you're creating fundamental conflicts for use of the feature in trigger functions or rules, w

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Marko Tiikkaja
On 2013-05-02 17:32, Karol Trzcionka wrote: W dniu 02.05.2013 17:17, Tom Lane pisze: It should in any case be possible to do this without converting them to reserved words; rather the implementation could be that those table aliases are made available when parsing the UPDATE RETURNING expression

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Karol Trzcionka
W dniu 02.05.2013 17:17, Tom Lane pisze: > It should in any case be possible to do this without converting them > to reserved words; rather the implementation could be that those table > aliases are made available when parsing the UPDATE RETURNING > expressions. (This is, in fact, the way that rule

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Tom Lane
David Fetter writes: > 1. As the SQL standard mandates that OLD and NEW be reserved words, we'll > re-reserve them. As I mentioned yesterday, I'm not exactly thrilled with re-reserving those, and especially not NEW as it is utterly unnecessary (since the default would already be to return the N

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread David Fetter
On Thu, May 02, 2013 at 11:04:15AM +0200, Karol Trzcionka wrote: > Hello, > I'm student who want to participate in Google Summer of Code. I want to > implement feature which allows to get old values directly from update > statement. I mean there should be possibility to choose the value > immedietl

[HACKERS] GSOC13 proposal - extend RETURNING syntax

2013-05-02 Thread Karol Trzcionka
Hello, I'm student who want to participate in Google Summer of Code. I want to implement feature which allows to get old values directly from update statement. I mean there should be possibility to choose the value immedietly before or after update in RETURNING statement. The syntax may be realized