Andres Freund andres at 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 bit
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
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, here
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
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 stage, but
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 be
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
On 2013-10-03 20:51:08 -0400, Robert Haas wrote:
On Thu, Oct 3, 2013 at 7:54 PM, Karol Trzcionka karl...@gmail.com 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
On Fri, Oct 4, 2013 at 4:42 AM, Karol Trzcionka karl...@gmail.com 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
On Fri, Oct 4, 2013 at 5:04 AM, Marko Tiikkaja ma...@joh.to 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;
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
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
On Thu, Oct 3, 2013 at 7:54 PM, Karol Trzcionka karl...@gmail.com 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
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]
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
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’:
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 in
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
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,
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
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
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
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);
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
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 the
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?
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 change
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 ]
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 da...@fetter.org
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
+++
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 it a
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
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
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
@@
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
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
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
immedietly
David Fetter da...@fetter.org 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
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 rules
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
Marko Tiikkaja ma...@joh.to 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
On 2013-05-02 12:23:19 -0400, Tom Lane wrote:
Marko Tiikkaja ma...@joh.to 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
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 ma...@joh.to 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
2013/5/2 Tom Lane t...@sss.pgh.pa.us
Marko Tiikkaja ma...@joh.to 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
David Fetter da...@fetter.org 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.
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
On Thu, May 02, 2013 at 01:40:59PM -0400, Tom Lane wrote:
David Fetter da...@fetter.org 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
Sent from my iPad
On 03-May-2013, at 0:07, David Fetter da...@fetter.org wrote:
On Thu, May 02, 2013 at 01:40:59PM -0400, Tom Lane wrote:
David Fetter da...@fetter.org writes:
On Thu, May 02, 2013 at 06:28:53PM +0200, Andres Freund wrote:
prior/after? Both are unreserved keywords atm and
David Fetter da...@fetter.org 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
Karol Trzcionka karl...@gmail.com 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
W dniu 02.05.2013 20:45, Tom Lane pisze:
David Fetter da...@fetter.org 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
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 ma...@joh.to writes:
What I'm more interested in is: how can we make this feature work in
PL/PgSQL where OLD means something
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
Karol Trzcionka karl...@gmail.com 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
54 matches
Mail list logo
