From: Mikhail Titov <mlt@gmx.us>
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [bug+patch] Inserting DEFAULT into generated columns from VALUES RTE
Date: Fri, 14 Aug 2020 14:57:38 -0500
Message-ID: <9q0o8nd6lvx.fsf@gmx.us> (raw)
In-Reply-To: <CAFj8pRC4Y2V3B0HjiPNcpwMfgd2Pj4bTJQJHtZuG=g2Nx5Puiw@mail.gmail.com>
References: <9q0sgcr416t.fsf@gmx.us>
<CAFj8pRC4Y2V3B0HjiPNcpwMfgd2Pj4bTJQJHtZuG=g2Nx5Puiw@mail.gmail.com>
Previously submitted patch got somehow trailing spaces mangled on the
way out. This is an attempt to use application/octet-stream MIME instead
of text/x-patch to preserve those for regression tests.
On Thu, Aug 13, 2020 at 12:11 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> please, assign your patch to commitfest application
Here is the backlink https://commitfest.postgresql.org/29/2681/
--
Mikhail
Attachments:download all (2 files, .tar.gz)v2-0001-Test-DEFAULT-in-VALUES-RTE-for-generated-columns.patch (application/octet-stream, 3.0K) download
From aa388025468b1c5ac7c2f3f5fc50ee09e63d5982 Mon Sep 17 00:00:00 2001
From: Mikhail Titov <mlt@gmx.us>
Date: Wed, 12 Aug 2020 22:42:37 -0500
Subject: [PATCH v2 1/2] Test DEFAULT in VALUES RTE for generated columns
---
src/test/regress/expected/generated.out | 9 +++++++++
src/test/regress/expected/identity.out | 3 +--
src/test/regress/sql/generated.sql | 4 ++++
src/test/regress/sql/identity.sql | 3 +--
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.outindex 7ccc3c65ed..31525ef2a6 100644--- a/src/test/regress/expected/generated.out+++ b/src/test/regress/expected/generated.out@@ -90,6 +90,15 @@ CREATE TABLE gtest_err_8 (a int PRIMARY KEY, b int GENERATED BY DEFAULT AS (a *
ERROR: for a generated column, GENERATED ALWAYS must be specified
LINE 1: ...E gtest_err_8 (a int PRIMARY KEY, b int GENERATED BY DEFAULT...
^
+-- test VALUES RTE with defaults+INSERT INTO gtest0 VALUES (1, DEFAULT), (2, DEFAULT);+SELECT * FROM gtest0;+ a | b +---+----+ 1 | 55+ 2 | 55+(2 rows)+
INSERT INTO gtest1 VALUES (1);
INSERT INTO gtest1 VALUES (2, DEFAULT);
INSERT INTO gtest1 VALUES (3, 33); -- error
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.outindex 7ac9df767f..b013a7c03a 100644--- a/src/test/regress/expected/identity.out+++ b/src/test/regress/expected/identity.out@@ -74,8 +74,7 @@ INSERT INTO itest2 DEFAULT VALUES;
INSERT INTO itest2 DEFAULT VALUES;
INSERT INTO itest3 DEFAULT VALUES;
INSERT INTO itest3 DEFAULT VALUES;
-INSERT INTO itest4 DEFAULT VALUES;-INSERT INTO itest4 DEFAULT VALUES;+INSERT INTO itest4 VALUES (DEFAULT, DEFAULT), (DEFAULT, DEFAULT);
SELECT * FROM itest1;
a | b
---+---
diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sqlindex 4cff1279c7..18bb1d3c78 100644--- a/src/test/regress/sql/generated.sql+++ b/src/test/regress/sql/generated.sql@@ -40,6 +40,10 @@ CREATE TABLE gtest_err_7d (a int PRIMARY KEY, b int GENERATED ALWAYS AS (generat
-- GENERATED BY DEFAULT not allowed
CREATE TABLE gtest_err_8 (a int PRIMARY KEY, b int GENERATED BY DEFAULT AS (a * 2) STORED);
+-- test VALUES RTE with defaults+INSERT INTO gtest0 VALUES (1, DEFAULT), (2, DEFAULT);+SELECT * FROM gtest0;+
INSERT INTO gtest1 VALUES (1);
INSERT INTO gtest1 VALUES (2, DEFAULT);
INSERT INTO gtest1 VALUES (3, 33); -- error
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sqlindex 1bf2a976eb..734014dc1c 100644--- a/src/test/regress/sql/identity.sql+++ b/src/test/regress/sql/identity.sql@@ -45,8 +45,7 @@ INSERT INTO itest2 DEFAULT VALUES;
INSERT INTO itest2 DEFAULT VALUES;
INSERT INTO itest3 DEFAULT VALUES;
INSERT INTO itest3 DEFAULT VALUES;
-INSERT INTO itest4 DEFAULT VALUES;-INSERT INTO itest4 DEFAULT VALUES;+INSERT INTO itest4 VALUES (DEFAULT, DEFAULT), (DEFAULT, DEFAULT);
SELECT * FROM itest1;
SELECT * FROM itest2;
--
2.28.0.windows.1
v2-0002-Allow-DEFAULT-in-VALUES-RTE-for-generated-columns.patch (application/octet-stream, 3.6K) download
From a745c91edbb0b59fddb4e7efb135f997c7830a2b Mon Sep 17 00:00:00 2001
From: Mikhail Titov <mlt@gmx.us>
Date: Wed, 12 Aug 2020 21:07:22 -0500
Subject: [PATCH v2 2/2] Allow DEFAULT in VALUES RTE for generated columns
Use T_SetToDefault in TLE instead of T_Var during parse analysis
as a marker for all DEFAULTs column. Let existing code to get rid of
that SetToDefault from TLE in INSERT rewriter, then force NULLs
during RTE column rewriting when attribute number was not set
as it normally would with Var.
Original patch was sent as text/x-patch and suffered whitespace mangling.
https://www.postgresql.org/message-id/flat/9q0sgcr416t.fsf@gmx.us
---
src/backend/parser/parse_relation.c | 50 ++++++++++++++++++++++------
src/backend/rewrite/rewriteHandler.c | 13 +++++++-
2 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.cindex b875a50646..baf05e2185 100644--- a/src/backend/parser/parse_relation.c+++ b/src/backend/parser/parse_relation.c@@ -2981,19 +2981,47 @@ expandNSItemVars(ParseNamespaceItem *nsitem,
if (colname[0])
{
Var *var;
+ ListCell *lc2;+ Node *std = NULL;+ bool all_default = true;
Assert(nscol->p_varno > 0);
- var = makeVar(nscol->p_varno,- nscol->p_varattno,- nscol->p_vartype,- nscol->p_vartypmod,- nscol->p_varcollid,- sublevels_up);- /* makeVar doesn't offer parameters for these, so set by hand: */- var->varnosyn = nscol->p_varnosyn;- var->varattnosyn = nscol->p_varattnosyn;- var->location = location;- result = lappend(result, var);+ foreach(lc2, nsitem->p_rte->values_lists)+ {+ List *row = (List*) lfirst(lc2);+ std = list_nth_node(Node, row, colindex);+ if (!IsA(std, SetToDefault))+ {+ all_default = false;+ break;+ }+ }++ if (all_default && std)+ {+ /*+ * If all nodes for the column are SetToDefault, keep the marker+ * for later INSERT VALUES list rewriting where we remove+ * TLE and force NULL constant values node while rewriting VALUES RTE.+ * We cannot just remove values column+ * as attribute numbering will be off.+ */+ result = lappend(result, std);+ }+ else+ {+ var = makeVar(nscol->p_varno,+ nscol->p_varattno,+ nscol->p_vartype,+ nscol->p_vartypmod,+ nscol->p_varcollid,+ sublevels_up);+ /* makeVar doesn't offer parameters for these, so set by hand: */+ var->varnosyn = nscol->p_varnosyn;+ var->varattnosyn = nscol->p_varattnosyn;+ var->location = location;+ result = lappend(result, var);+ }
if (colnames)
*colnames = lappend(*colnames, colnameval);
}
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.cindex fe777c3103..f885fe7405 100644--- a/src/backend/rewrite/rewriteHandler.c+++ b/src/backend/rewrite/rewriteHandler.c@@ -1373,8 +1373,19 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
Form_pg_attribute att_tup;
Node *new_expr;
+ /*+ * TLE was likely already removed for this generated column.+ * We do not have a good way to check it now,+ * but attrno should not be 0 anyway.+ */
if (attrno == 0)
- elog(ERROR, "cannot set value in column %d to DEFAULT", i);+ {+ SetToDefault *std = (SetToDefault*) col;+ new_expr = (Node*) makeNullConst(std->typeId, std->typeMod, std->collation);+ newList = lappend(newList, new_expr);+ continue;+ }+
att_tup = TupleDescAttr(target_relation->rd_att, attrno - 1);
if (!force_nulls && !att_tup->attisdropped)
--
2.28.0.windows.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: mlt@gmx.us, pgsql-hackers@lists.postgresql.org
Subject: Re: [bug+patch] Inserting DEFAULT into generated columns from VALUES RTE
In-Reply-To: <9q0o8nd6lvx.fsf@gmx.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox