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
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, h
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 sta
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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.
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
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
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
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 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
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 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
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:
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
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/
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
54 matches
Mail list logo