Hello! According to the docs[1], one may use DEFAULT keyword while inserting into generated columns (stored and identity). However, currently it works only for a single VALUES sublist with DEFAULT for a generated column but not for the case when VALUES RTE is used. This is not being tested and it is broken.
I am attaching two patches. One for tests and another one with the proposed changes based on ideas from Andrew on IRC. So if all good there goes the credit where credit is due. If patch is no good, then it is likely my misunderstanding how to put words into code :-) This is my only second patch to PostgreSQL (the first one was rejected) so don't be too harsh :-) It may not be perfect but I am open for a feedback and this is just to get the ball rolling and to let the community know about this issue. Before you ask why would I want to insert DEFAULTs ... well, there are ORMs[2] that still need to be patched and current situation contradicts documentation[1]. Footnotes: [1] https://www.postgresql.org/docs/devel/ddl-generated-columns.html [2] https://github.com/rails/rails/pull/39368#issuecomment-670351379 -- Mikhail
From d606c4f952a6080dff6fb621ea034bfce2865f7b Mon Sep 17 00:00:00 2001 From: Mikhail Titov <m...@gmx.us> Date: Wed, 12 Aug 2020 22:42:37 -0500 Subject: [PATCH 1/2] Test DEFAULT in VALUES RTE for generated columns --- src/test/regress/expected/generated.out | 9 +++++++++ src/test/regress/expected/identity.out | 13 +++++++++---- src/test/regress/sql/generated.sql | 4 ++++ src/test/regress/sql/identity.sql | 1 + 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out index 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.out index 7ac9df767f..ca27b7ff73 100644 --- a/src/test/regress/expected/identity.out +++ b/src/test/regress/expected/identity.out @@ -76,6 +76,7 @@ INSERT INTO itest3 DEFAULT VALUES; INSERT INTO itest3 DEFAULT VALUES; INSERT INTO itest4 DEFAULT VALUES; INSERT INTO itest4 DEFAULT VALUES; +INSERT INTO itest4 (a, b) VALUES (DEFAULT, 'foo'), (DEFAULT, 'bar'); SELECT * FROM itest1; a | b ---+--- @@ -99,10 +100,12 @@ SELECT * FROM itest3; SELECT * FROM itest4; a | b ----+--- +---+----- 1 | 2 | -(2 rows) + 3 | foo + 4 | bar +(4 rows) -- VALUES RTEs INSERT INTO itest3 VALUES (DEFAULT, 'a'); @@ -211,11 +214,13 @@ ALTER TABLE itest4 ALTER COLUMN a DROP NOT NULL; INSERT INTO itest4 DEFAULT VALUES; SELECT * FROM itest4; a | b ----+--- +---+----- 1 | 2 | + 3 | foo + 4 | bar | -(3 rows) +(5 rows) -- check that sequence is removed SELECT sequence_name FROM itest4_a_seq; diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql index 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.sql index 1bf2a976eb..b3d99583c2 100644 --- a/src/test/regress/sql/identity.sql +++ b/src/test/regress/sql/identity.sql @@ -47,6 +47,7 @@ INSERT INTO itest3 DEFAULT VALUES; INSERT INTO itest3 DEFAULT VALUES; INSERT INTO itest4 DEFAULT VALUES; INSERT INTO itest4 DEFAULT VALUES; +INSERT INTO itest4 (a, b) VALUES (DEFAULT, 'foo'), (DEFAULT, 'bar'); SELECT * FROM itest1; SELECT * FROM itest2; -- 2.28.0.windows.1
From 7a187f698d31638c65da10a71e97060793a29c7f Mon Sep 17 00:00:00 2001 From: Mikhail Titov <m...@gmx.us> Date: Wed, 12 Aug 2020 21:07:22 -0500 Subject: [PATCH 2/2] Allow DEFAULT in VALUES RTE for generated columns --- 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.c index b875a50646..da26863d22 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 *c; + 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(c, nsitem->p_rte->values_lists) + { + List *row = (List*) lfirst(c); + 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. + * 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.c index 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