Re: [HACKERS] [pgsql-advocacy] GSoC 2014 - mentors, students and admins

[Karol Trzcionka]

W dniu 27.02.2014 22:25, Thom Brown pisze:
 On 27 February 2014 21:08, David Fetter da...@fetter.org
 mailto:da...@fetter.org wrote:

 For MADlib, no.  Are you asking for mentors in general?


 Ah yes, I should clarify.  Yes, mentors in general.
In general I can help but I'm not sure if I'm not too fresh in pgsql ;)
However after GSOC as student I can try the another side.
Regards,
Karol

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 be able to make AFTER.x translate to a Var
 referencing that range table entry.
It's not enough. Even if we know where we are, there are more issues.
The main question is: how should we pass information about hello, I'm
specific Var, don't evaluate me like others? We can add two fields to
Var structure (flag - normal/before/after and no. column) - however it
needs to modify copyObject for Vars (at now it's done e.g. in
flatten_join_alias_vars_mutator for varoattno and varnoold). If
copyObject is modified, sure code in
flatten_join_alias_vars_mutator/pullup_replace_vars_callback will be
useless. I don't know if modifying pg at the low-level (changing
structure of Var and behaviour of copyObject) is good idea. Yes if the
community really want it but it needs more votes. There is medium
solution: changing Var structure and do the copy like now (in mutator
and callback) but w/o the condition statement (for the new fields). I
think it might need to modify more places in code because of comparing
vars (maybe we'd need to include new fields while comparision).
Regards,
Karol Trzcionka


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 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)
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, I only point at the difference).
 Now this is a hack, because we don't really want to add an extra
 scan/join just to get the behavior we want.  But it seems to me
 significant that this processing makes Vars that refer to the target
 table refer to the new values, and if we got rid of it, they'd refer
 to the old values.  Can't we contrive to make AFTER.x parse into the
 same Var node that x currently does?  Then we don't need an RTE for
 it.  And maybe BEFORE.x ought to parse to the same node that just
 plain x does but with some marking, or some other node wrapped around
 it (like a TargetEntry with some flag set?) that suppresses this
 processing.  I'm just shooting from the hip here; that might be wrong
 in detail, or even in broad strokes, but it just looks to me like the
 additional RTE kind is going to bleed into a lot of places.
While planning/analyzing the problem there were many ideas about hot to
solve it. I was trying to avoid adding new RTE and referencing to core
table. However it makes more and more issues. You can see some PoC on
the
https://github.com/davidfetter/postgresql_projects/compare/returning_before_after
(other ideas I revert w/o commit because I couldn't get expected
result). The other major reason was that we can avoid touching executor
and/or parser's core (flex) this way. One observation: why shouldn't we
use the values computed at the moment (it would be computed again if we
want to do it later, in executor)?
I think we can do it by modify the Var structure (add some kind of flag
while generating the vars in parser?) but I'm not sure if it is good
idea. The major issue is to know if the Var/TargetEntry references to
the real alias BEFORE (named with AS syntax or even the real
table-name - I can see there is no difference in code) or the virtual
(from feature patch) BEFORE. Doing it in parser (more low-level)
would be very awful - we'd need to check in which part of statement
BEFORE/AFTER is placed (it is not allowed to use it in the other places
than in RETURNING). We don't want to make BEFORE and AFTER
restricted keywords.
Now most of the code means don't touch these because they are not real :)
If anyone has the fresh idea to it better, please write it by mail, I
don't have more ideas how to solve it.
 This patch also has significant style issues.
I'll try to fix it soon.
Regards,
Karol Trzcionka



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[Karol Trzcionka]

 if there are more than one matching RangeTblEntry for before
 and/or for after? Is it an error condition or of them should be fixed?
I think it is safe, it is the first and the last index. On each level of
statement there can be (at most) the only one before and one after
alias.
Regards,
Karol Trzcionka
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 ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo-ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo-ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo-ri_TrigDesc-trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, planSlot);
 
 		if (slot == NULL)		/* do nothing */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo-ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 908f397..3485bd7 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1987,6 +1987,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_ALIAS:
 /* nothing to do */
 break;
 			case RTE_SUBQUERY:
@@ -2701,6 +2702,7 @@ range_table_mutator(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_ALIAS:
 /* we don't bother to copy eref, aliases, etc; OK? */
 break;
 			case

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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_targetlist
- Add all before variables to targetlist
- Avoid adding variables to slot for AFTER.
- Treat varnoold like a flag - prevent from adjustment if RTE_BEFORE
- All before/after are now set on OUTER_VAR
- Rename fix_varno_varattno to bind_returning_variables
- Add comment about bind_returning_variables
- Remove unneeded code in fix_join_expr_mutator (it was changing varno
of RTE_BEFORE - now there is not any var with varno assigned to it)
Regards,
Karol Trzcionka
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 ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo-ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo-ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo-ri_TrigDesc-trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, planSlot);
 
 		if (slot == NULL)		/* do nothing */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo-ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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
 uninitialized issue
 - Revert changing varno in add_vars_to_targetlist
 - Add all before variables to targetlist
 - Avoid adding variables to slot for AFTER.
 - Treat varnoold like a flag - prevent from adjustment if RTE_BEFORE
 - All before/after are now set on OUTER_VAR
 - Rename fix_varno_varattno to bind_returning_variables
 - Add comment about bind_returning_variables
 - Remove unneeded code in fix_join_expr_mutator (it was changing varno
 of RTE_BEFORE - now there is not any var with varno assigned to it)
I've just realized the prepare_returning_before() is unneeded right now
so I've removed it. Version 7, hopefully the last. ;)
Regards,
Karol Trzcionka
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 ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo-ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo-ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo-ri_TrigDesc-trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, planSlot);
 
 		if (slot == NULL)		/* do nothing */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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);
   ^
 setrefs.c:1957:21: note: ‘before_index’ was declared here
   int after_index=0, before_index;
  ^
Right, my mistake. Sorry and thanks. Fixed.
Regards,
Karol Trzcionka
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 ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo-ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo-ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo-ri_TrigDesc-trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, planSlot);
 
 		if (slot == NULL)		/* do nothing */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo-ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 908f397..461ec4f 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1987,6 +1987,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* nothing to do */
 break;
 			case

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 the rejected chunk of the
 patch.
It fails because the HINT was changed, fixed.
That version merges some nested ifs left over from earlier work.
Regards,
Karol Trzcionka
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 ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo-ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo-ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo-ri_TrigDesc-trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, planSlot);
 
 		if (slot == NULL)		/* do nothing */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo-ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 908f397..461ec4f 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1987,6 +1987,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* nothing to do */
 break;
 			case RTE_SUBQUERY:
@@ -2701,6 +2702,7

Re: [HACKERS] Patch to add support of IF NOT EXISTS to others CREATE statements

[Karol Trzcionka]

W dniu 26.07.2013 02:44, Fabrízio de Royes Mello pisze:
 Should be... I fix that in attached patch.
Hello, as I can see there are more inconsistent places.
First style:
OperatorCreate
---
Second style:
ProcedureCreate
TypeCreate
DefineTSParser
DefineType
DefineEnum
---
Third style:
CreateCast
DefineTSDictionary
DefineTSTemplate
DefineTSConfiguration
DefineRange
DefineCompositeType
Regards,
Karol


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch to add support of IF NOT EXISTS to others CREATE statements

[Karol Trzcionka]

Hello,
patch works fine but is there any reason to comparing each ifNotExists
in different way?
i.e.
ProcedureCreate
if (!ifNotExists)
...
else
{
...
return
}

TypeCreate
if (ifNotExists)
{
...
return
}
...
---
Shouldn't it be more consistent?
Regards,
Karol


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 change var-varno to base RTE (it should
always exists, the value is temporary, it will change to OUTER_VAR)
- if it is a before - add to targetlist new var independently from
rel-attr_needed[attno].
Additionally I've change build_joinrel_tlist to ignore any BEFORE
RTEs. The regression tests are updated.
Regards,
Karol


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo-ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo-ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo-ri_TrigDesc-trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, planSlot);
 
 		if (slot == NULL)		/* do nothing */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo-ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index a896d76..409b4d1 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1928,6 +1928,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* nothing to do */
 break;
 			case RTE_SUBQUERY:
@@ -2642,6 +2643,7 @@ range_table_mutator(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* we don't bother to copy eref, aliases, etc; OK? */
 break;
 			case RTE_SUBQUERY:
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 48cd9dc..79b03af 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2366,6 +2366,7 @@ 

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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/src/sgml/ref/update.sgml
@@ -194,12 +194,27 @@ UPDATE [ ONLY ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo-ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo-ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 15f5dcc..06ebaf3 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -603,7 +603,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo-ri_TrigDesc-trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, planSlot);
 
 		if (slot == NULL)		/* do nothing */
 			return NULL;
@@ -737,6 +737,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo-ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index a896d76..409b4d1 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1928,6 +1928,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* nothing to do */
 break;
 			case RTE_SUBQUERY:
@@ -2642,6 +2643,7 @@ range_table_mutator(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* we don't bother to copy eref, aliases, etc; OK? */
 break;
 			case RTE_SUBQUERY:
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 48cd9dc..79b03af 100644
--- 

Re: [HACKERS] [v9.4] row level security

[Karol Trzcionka]

Current head 4cbe3ac3e86790d05c569de4585e5075a62a9b41 - patch applies
correct (only change needed in parallel_schedule).
However it fails on own regression tests (other tests pass).
Regards,
Karol


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] compiler warning in UtfToLocal and LocalToUtf (conv.c)

[Karol Trzcionka]

Hello,
in the current master head (4cbe3ac3e86790d05c569de4585e5075a62a9b41),
I've noticed the compiler warnings in src/backend/utils/mb/conv.c
conv.c: In function ‘UtfToLocal’:
conv.c:252:23: error: ‘iutf’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
...
conv.c: In function ‘LocalToUtf’:
conv.c:301:23: error: ‘iiso’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
...
The compiler doesn't know that the 'l' may varies between 1 and 4. Hot
fix may be:
1. preinitialize it
2. delete last if statement (change else-if to else)
3. change it to switch-case and set default behaviour
Regards,
Karol


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] compiler warning in UtfToLocal and LocalToUtf (conv.c)

[Karol Trzcionka]

W dniu 19.07.2013 02:42, Tom Lane pisze:
 Hm, what compiler version are you using? I've never seen such a
 warning (and that code hasn't changed in some time). 
gcc version 4.8.1 20130612 (Red Hat 4.8.1-2) (GCC)
Regards,
Karol


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 tuple after all updates done between planner and
executor (in fact it is not planSlot right now). I don't know whether
the tuple could be deleted in the intervening time and if the pointer
doesn't point to unreserved memory (I mean - memory which may be
overwritten by something meanwhile).
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 ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d86e9ad..fafd311 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2335,7 +2335,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 TupleTableSlot *
 ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	 ResultRelInfo *relinfo,
-	 ItemPointer tupleid, TupleTableSlot *slot)
+	 ItemPointer tupleid, TupleTableSlot *slot, TupleTableSlot **planSlot)
 {
 	TriggerDesc *trigdesc = relinfo-ri_TrigDesc;
 	HeapTuple	slottuple = ExecMaterializeSlot(slot);
@@ -2381,6 +2381,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo-ri_junkFilter, newSlot);
+		*planSlot = newSlot;
 		slottuple = ExecMaterializeSlot(slot);
 		newtuple = slottuple;
 	}
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index e934c7b..27859dd 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -599,7 +599,7 @@ ExecUpdate(ItemPointer tupleid,
 		resultRelInfo-ri_TrigDesc-trig_update_before_row)
 	{
 		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-	tupleid, slot);
+	tupleid, slot, planSlot);
 
 		if (slot == NULL)		/* do nothing */
 			return NULL;
@@ -733,6 +733,7 @@ lreplace:;
 		   hufd.xmax);
 	if (!TupIsNull(epqslot))
 	{
+		planSlot = epqslot;
 		*tupleid = hufd.ctid;
 		slot = ExecFilterJunk(resultRelInfo-ri_junkFilter, epqslot);
 		tuple = ExecMaterializeSlot(slot);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 42d6621..cc68568 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1920,6 +1920,7 @@ range_table_walker(List *rtable,
 		{
 			case 

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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,12 +194,27 @@ UPDATE [ ONLY ] replaceable class=PARAMETERtable_name/replaceable [ * ] [
 termreplaceable class=PARAMETERoutput_expression/replaceable/term
 listitem
  para
-  An expression to be computed and returned by the commandUPDATE/
-  command after each row is updated.  The expression can use any
-  column names of the table named by replaceable class=PARAMETERtable_name/replaceable
-  or table(s) listed in literalFROM/.
-  Write literal*/ to return all columns.
+  An expression to be computed and returned by the
+  commandUPDATE/ command either before or after (prefixed with
+  literalBEFORE./literal and literalAFTER./literal,
+  respectively) each row is updated.  The expression can use any
+  column names of the table named by replaceable
+  class=PARAMETERtable_name/replaceable or table(s) listed in
+  literalFROM/.  Write literalAFTER.*/literal to return all 
+  columns after the update. Write literalBEFORE.*/literal for all
+  columns before the update. Write literal*/literal to return all
+  columns after update and all triggers fired (these values are in table
+  after command). You may combine BEFORE, AFTER and raw columns in the
+  expression.
  /para
+ warningpara
+ Mixing table names or aliases named before or after with the
+ above will result in confusion and suffering.  If you happen to
+ have a table called literalbefore/literal or
+ literalafter/literal, alias it to something else when using
+ RETURNING.
+ /para/warning
+
 /listitem
/varlistentry
 
@@ -287,15 +302,16 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   /para
 
   para
-   Perform the same operation and return the updated entries:
+   Perform the same operation and return information on the changed entries:
 
 programlisting
 UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   WHERE city = 'San Francisco' AND date = '2003-07-03'
-  RETURNING temp_lo, temp_hi, prcp;
+  RETURNING temp_lo AS new_low, temp_hi AS new_high, BEFORE.temp_hi/BEFORE.temp_low AS old_ratio, AFTER.temp_hi/AFTER.temp_low AS new_ratio prcp;
 /programlisting
   /para
 
+
   para
Use the alternative column-list syntax to do the same update:
 programlisting
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 42d6621..cc68568 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1920,6 +1920,7 @@ range_table_walker(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* nothing to do */
 break;
 			case RTE_SUBQUERY:
@@ -2622,6 +2623,7 @@ range_table_mutator(List *rtable,
 		{
 			case RTE_RELATION:
 			case RTE_CTE:
+			case RTE_BEFORE:
 /* we don't bother to copy eref, aliases, etc; OK? */
 break;
 			case RTE_SUBQUERY:
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index b2183f4..2698535 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2351,6 +2351,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
 	switch (node-rtekind)
 	{
 		case RTE_RELATION:
+		case RTE_BEFORE:
 			WRITE_OID_FIELD(relid);
 			WRITE_CHAR_FIELD(relkind);
 			break;
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 3a16e9d..04931ee 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1189,6 +1189,7 @@ _readRangeTblEntry(void)
 	switch (local_node-rtekind)
 	{
 		case RTE_RELATION:
+		case RTE_BEFORE:
 			READ_OID_FIELD(relid);
 			READ_CHAR_FIELD(relkind);
 			break;
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 839ed9d..7fc6ea1 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -174,9 +174,16 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars,
 		if (IsA(node, Var))
 		{
 			Var		   *var = (Var *) node;
-			RelOptInfo *rel = find_base_rel(root, var-varno);
+			RelOptInfo *rel;
 			int			attno = var-varattno;
+			RangeTblEntry *rte;
 
+			if (root-parse-commandType == CMD_UPDATE){
+rte = ((RangeTblEntry *) list_nth(root-parse-rtable, (var-varno)-1));
+if(rte-rtekind == RTE_BEFORE)
+	continue;
+			}
+			rel = find_base_rel(root, var-varno);
 			Assert(attno = rel-min_attr  attno = rel-max_attr);
 			attno -= rel-min_attr;
 			if (rel-attr_needed[attno] == NULL)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index d80c264..77b0c38 100644
--- a/src/backend/optimizer/plan/planner.c
+++ 

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 switch with
current * behaviour - I don't know yet)
- patch passes all regression tests
- the code is uncommented already, to be fixed
I'm not sure what may fail so you use it on your risk ;)
Regards,
Karol
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index b2183f4..2698535 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2351,6 +2351,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
 	switch (node-rtekind)
 	{
 		case RTE_RELATION:
+		case RTE_BEFORE:
 			WRITE_OID_FIELD(relid);
 			WRITE_CHAR_FIELD(relkind);
 			break;
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 3a16e9d..04931ee 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1189,6 +1189,7 @@ _readRangeTblEntry(void)
 	switch (local_node-rtekind)
 	{
 		case RTE_RELATION:
+		case RTE_BEFORE:
 			READ_OID_FIELD(relid);
 			READ_CHAR_FIELD(relkind);
 			break;
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 839ed9d..7fc6ea1 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -174,9 +174,16 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars,
 		if (IsA(node, Var))
 		{
 			Var		   *var = (Var *) node;
-			RelOptInfo *rel = find_base_rel(root, var-varno);
+			RelOptInfo *rel;
 			int			attno = var-varattno;
+			RangeTblEntry *rte;
 
+			if (root-parse-commandType == CMD_UPDATE){
+rte = ((RangeTblEntry *) list_nth(root-parse-rtable, (var-varno)-1));
+if(rte-rtekind == RTE_BEFORE)
+	continue;
+			}
+			rel = find_base_rel(root, var-varno);
 			Assert(attno = rel-min_attr  attno = rel-max_attr);
 			attno -= rel-min_attr;
 			if (rel-attr_needed[attno] == NULL)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index d80c264..77b0c38 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1989,6 +1989,9 @@ preprocess_rowmarks(PlannerInfo *root)
 		if (rte-relkind == RELKIND_FOREIGN_TABLE)
 			continue;
 
+		if (rte-relkind == RELKIND_BEFORE)
+			continue;
+
 		rels = bms_del_member(rels, rc-rti);
 
 		newrc = makeNode(PlanRowMark);
@@ -2028,6 +2031,9 @@ preprocess_rowmarks(PlannerInfo *root)
 		if (!bms_is_member(i, rels))
 			continue;
 
+		if (rte-relkind == RELKIND_BEFORE)
+			continue;
+
 		newrc = makeNode(PlanRowMark);
 		newrc-rti = newrc-prti = i;
 		newrc-rowmarkId = ++(root-glob-lastRowMarkId);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index b78d727..6828a7d 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1691,6 +1691,12 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
 	if (IsA(node, Var))
 	{
 		Var		   *var = (Var *) node;
+		if (var-varno=list_length(context-root-parse-rtable)  var-varno1  context-root-parse-commandType == CMD_UPDATE){
+			RangeTblEntry *rte_a,*rte_b;
+			rte_a = (RangeTblEntry *)list_nth(context-root-parse-rtable,var-varno-1);
+			rte_b = (RangeTblEntry *)list_nth(context-root-parse-rtable,var-varno-2);
+			if (rte_a-rtekind == RTE_BEFORE  rte_b-rtekind == RTE_BEFORE) var-varno-=1;
+		}
 
 		/* First look for the var in the input tlists */
 		newvar = search_indexed_tlist_for_var(var,
@@ -1892,6 +1898,37 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context)
  *
  * Note: resultRelation is not yet adjusted by rtoffset.
  */
+
+void fix_varno_varattno(List *rlist, int begin, int bef, int aft){
+	ListCell   *temp;
+	ListCell   *temp2;
+	Var *var;
+	foreach(temp, rlist){
+		TargetEntry *tle = (TargetEntry *)lfirst(temp);
+
+		if(IsA(tle, TargetEntry)){
+			var = (Var*)tle-expr;
+		}else if(IsA(tle, Var)) var=(Var*)tle;
+		if( IsA(var, Var) ){
+			if(var-varnoold == bef){
+var-varno = OUTER_VAR;
+var-varattno = var-varoattno + begin;
+			}
+			else if(var-varnoold == aft)
+			{
+ var-varno = OUTER_VAR;
+ var-varattno = var-varoattno;
+			}
+		}
+		else if( IsA(var, OpExpr )){
+			fix_varno_varattno(((OpExpr*)var)-args, begin, bef, aft);
+		}
+		else if( IsA(var, FuncExpr )){
+			fix_varno_varattno(((FuncExpr*)var)-args, begin, bef, aft);
+		}
+	}
+}
+
 static List *
 set_returning_clause_references(PlannerInfo *root,
 List *rlist,
@@ -1900,7 +1937,48 @@ set_returning_clause_references(PlannerInfo *root,
 int rtoffset)
 {
 	indexed_tlist *itlist;
+	int before_index=0, after_index=0, var_index=0;
+	Query  *parse = root-parse;
+
+	ListCell   *rt;
+	RangeTblEntry *bef;
 
+	TargetEntry *tr;
+
+	int index_rel=1;
+	int index_var=0;
+
+
+	

[HACKERS] RETURNING syntax for COPY

[Karol Trzcionka]

Hello,
as a continuation of my proposal expanding RETURNING syntax by
AFTER/BEFORE, there can be enough time to implement RETURNING for COPY.
I'd like to hear your opinion on that. My draft idea is:
COPY FROM ... RETURNING table_name.* - returns all values copied to
table after all triggers invoke
COPY FROM ... RETURNING BEFORE.* - returns all values copied to table
before triggers (all raw values from file)
COPY TO ... RETURNING table_name.* - returns all values from table
COPY TO ... RETURNING AFTER.*/BEFORE.* - the same
What do you think about?
Regards,
Karol Trzcionka


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 as aliases. That means: OLD keywordswould be alias to row
before update and NEW to row after update. The conclusion of syntax is:
UPDATE foo SET bar=bar+1 RETURNING OLD.bar AS old_bar, NEW.bar AS new_bar;
UPDATE foo SET ... RETURNING NEW.* will be equivalent to UPDATE foo SET
... RETURNING foo.*
It may be possible to add similar syntax to DELETE and INSERT statements
but I'm not sure if it makes a sense (OLD for DELETE will be alias to
row before delete, NEW for INSERT will be alias to row after insert and
all triggers - however what about NEW for delete and OLD for INSERT?).
Additionally NEW and OLD values will be reserved keywords (it might be
some capability problem since in new PostgreSQL it isn't reserved -
however standard says it is and in old PgSQL it was).
I'd like to hear (read) yours feedback about syntax and/or implement
issues related to this proposal.
Regards,
Karol Trzcionka


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 rules use these names
 now.) Probably it should work something like add these aliases if
 they don't already exist in the query, so as to avoid breaking
 existing applications. I don't really see a lot of value in hacking
 the behavior of either INSERT RETURNING or DELETE RETURNING. regards,
 tom lane 
I'm not sure about it. If it is not reserved keyword how can user get
old value from table named old. The new value is not a problem
(doesn't conflict) but what should happened in statement:
UPDATE old SET foo=foo+1 RETURNING old.foo;
If it returns old value, it'll break capability. If it returns new value
(as now), there is no possibility to get old value (and user cries
because of broken feature).
Regards,
Karol Trzcionka


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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 statements.
Regards,
Karol Trzcionka


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[Karol Trzcionka]

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
 I think they're best off being handled as automatically-supplied
 aliases.  IOW the patch needn't touch gram.y at all.
Well... the syntax which wouldn't conflict with Pl/PgSQL is (draft):
INSERT INTO foo ... RETURNING AFTER.*
UPDATE foo SET ... RETURNING AFTER.*, BEFORE.*
DELETE FROM foo ... RETURNING BEFORE.*
It will not solve the problems:
1. How to access to old rows if the table is named BEFORE?
2. Should AFTER for DELETE and BEFORE for INSERT be allowed? If yes what
should they return? I mean: what should be result of:
INSERT INTO foo ... RETURNING BEFORE.*
and
DELETE FROM foo ... RETURNING AFTER.*
?
The best summarize of dropping NEW/OLD keywords for this proposal was
proposed while IRC meeting:
create or replace function ft1(integer) returns integer language plpgsql
as $f$ x declare r rt1; begin select 1 as a into r; update rt1 r set
a = a + 1 returning XXX into r; raise notice 'r = %', r; return null;
end; $f$;
Regards,
Karol Trzcionka


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax

[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