Convert ExecComputeStoredGenerated to use tuple slots

This code was still using the old style of forming a heap tuple rather
than using tuple slots.  This would be less efficient if a non-heap
access method was used.  And using tuple slots is actually quite a bit
faster when using heap as well.

Also add some test cases for generated columns with null values and
with varlena values.  This lack of coverage was discovered while
working on this patch.

Discussion: https://www.postgresql.org/message-id/flat/20190331025744.ugbsyks7czfcoksd%40alap3.anarazel.de

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index d545bbce8a29fb117417905e46969cadac454d23..d3a0dece5ad5ad56b4fdef337d6cc0e28b89cc18 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -53,6 +53,7 @@
 #include "storage/bufmgr.h"
 #include "storage/lmgr.h"
 #include "utils/builtins.h"
+#include "utils/datum.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
 
@@ -254,9 +255,6 @@ ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot)
        MemoryContext oldContext;
        Datum      *values;
        bool       *nulls;
-       bool       *replaces;
-       HeapTuple       oldtuple, newtuple;
-       bool            should_free;
 
        Assert(tupdesc->constr && tupdesc->constr->has_generated_stored);
 
@@ -294,11 +292,15 @@ ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot)
 
        values = palloc(sizeof(*values) * natts);
        nulls = palloc(sizeof(*nulls) * natts);
-       replaces = palloc0(sizeof(*replaces) * natts);
+
+       slot_getallattrs(slot);
+       memcpy(nulls, slot->tts_isnull, sizeof(*nulls) * natts);
 
        for (int i = 0; i < natts; i++)
        {
-               if (TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_STORED)
+               Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
+
+               if (attr->attgenerated == ATTRIBUTE_GENERATED_STORED)
                {
                        ExprContext *econtext;
                        Datum           val;
@@ -311,20 +313,19 @@ ExecComputeStoredGenerated(EState *estate, TupleTableSlot *slot)
 
                        values[i] = val;
                        nulls[i] = isnull;
-                       replaces[i] = true;
+               }
+               else
+               {
+                       if (!nulls[i])
+                               values[i] = datumCopy(slot->tts_values[i], attr->attbyval, attr->attlen);
                }
        }
 
-       oldtuple = ExecFetchSlotHeapTuple(slot, true, &should_free);
-       newtuple = heap_modify_tuple(oldtuple, tupdesc, values, nulls, replaces);
-       /*
-        * The tuple will be freed by way of the memory context - the slot might
-        * only be cleared after the context is reset, and we'd thus potentially
-        * double free.
-        */
-       ExecForceStoreHeapTuple(newtuple, slot, false);
-       if (should_free)
-               heap_freetuple(oldtuple);
+       ExecClearTuple(slot);
+       memcpy(slot->tts_values, values, sizeof(*values) * natts);
+       memcpy(slot->tts_isnull, nulls, sizeof(*nulls) * natts);
+       ExecStoreVirtualTuple(slot);
+       ExecMaterializeSlot(slot);
 
        MemoryContextSwitchTo(oldContext);
 }

diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index d4ed3f7ae1bed26aa746fe2f13d2d58ad20cdb5b..f62c93f4686c51b63e85156a6701f9cb3ef9e4b9 100644 (file)
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -226,15 +226,16 @@ NOTICE:  merging multiple inherited definitions of column "b"
 ERROR:  inherited column "b" has a generation conflict
 DROP TABLE gtesty;
 -- test stored update
-CREATE TABLE gtest3 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 3) STORED);
-INSERT INTO gtest3 (a) VALUES (1), (2), (3);
+CREATE TABLE gtest3 (a int, b int GENERATED ALWAYS AS (a * 3) STORED);
+INSERT INTO gtest3 (a) VALUES (1), (2), (3), (NULL);
 SELECT * FROM gtest3 ORDER BY a;
  a | b 
 ---+---
  1 | 3
  2 | 6
  3 | 9
-(3 rows)
+   |  
+(4 rows)
 
 UPDATE gtest3 SET a = 22 WHERE a = 2;
 SELECT * FROM gtest3 ORDER BY a;
@@ -243,7 +244,29 @@ SELECT * FROM gtest3 ORDER BY a;
   1 |  3
   3 |  9
  22 | 66
-(3 rows)
+    |   
+(4 rows)
+
+CREATE TABLE gtest3a (a text, b text GENERATED ALWAYS AS (a || '+' || a) STORED);
+INSERT INTO gtest3a (a) VALUES ('a'), ('b'), ('c'), (NULL);
+SELECT * FROM gtest3a ORDER BY a;
+ a |  b  
+---+-----
+ a | a+a
+ b | b+b
+ c | c+c
+   | 
+(4 rows)
+
+UPDATE gtest3a SET a = 'bb' WHERE a = 'b';
+SELECT * FROM gtest3a ORDER BY a;
+ a  |   b   
+----+-------
+ a  | a+a
+ bb | bb+bb
+ c  | c+c
+    | 
+(4 rows)
 
 -- COPY
 TRUNCATE gtest1;

diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql
index da11b8c9b89a80bf2abb70aceb046b0ff2dcd324..6a56ae260f6ff8f6c92ebf1e4bf3f6dc6ea69860 100644 (file)
--- a/src/test/regress/sql/generated.sql
+++ b/src/test/regress/sql/generated.sql
@@ -95,12 +95,18 @@ CREATE TABLE gtest1_2 () INHERITS (gtest1, gtesty);  -- error
 DROP TABLE gtesty;
 
 -- test stored update
-CREATE TABLE gtest3 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 3) STORED);
-INSERT INTO gtest3 (a) VALUES (1), (2), (3);
+CREATE TABLE gtest3 (a int, b int GENERATED ALWAYS AS (a * 3) STORED);
+INSERT INTO gtest3 (a) VALUES (1), (2), (3), (NULL);
 SELECT * FROM gtest3 ORDER BY a;
 UPDATE gtest3 SET a = 22 WHERE a = 2;
 SELECT * FROM gtest3 ORDER BY a;
 
+CREATE TABLE gtest3a (a text, b text GENERATED ALWAYS AS (a || '+' || a) STORED);
+INSERT INTO gtest3a (a) VALUES ('a'), ('b'), ('c'), (NULL);
+SELECT * FROM gtest3a ORDER BY a;
+UPDATE gtest3a SET a = 'bb' WHERE a = 'b';
+SELECT * FROM gtest3a ORDER BY a;
+
 -- COPY
 TRUNCATE gtest1;
 INSERT INTO gtest1 (a) VALUES (1), (2);