Re: [bug+patch] Inserting DEFAULT into generated columns from VALUES RTE - Mailing list pgsql-hackers

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> I think it's actually easier to just do it all in the rewriter -- at
> the point where we see that we're about to insert potentially illegal
> values from a VALUES RTE into a generated column, scan it to see if
> all the values in that column are DEFAULTs, and if so trigger the
> existing code to replace the Var in the tlist with the generated
> column expression. That way we only do extra work in the case for
> which we're currently throwing an error, rather than for every query.

That makes sense, and it leads to a nicely small patch.  I reviewed
this and pushed it.  I found only one nitpicky bug: in
findDefaultOnlyColumns, the test must be bms_is_empty(default_only_cols)
not just default_only_cols == NULL, or it will fail to fall out early
as intended when the first row contains some DEFAULTs but later rows
don't.  I did tweak some of the commentary, too.

> I haven't touched the existing error messages. I think that's a
> subject for a separate patch.

Fair.  After looking around a bit, I think that getting an error
cursor as I'd speculated about is more trouble than it's worth.
For starters, we'd have to pass down the query string into this
code, and there might be some ticklish issues about whether a given
chunk of parsetree came from that string or from some rule or view.
However, I think that just adjusting the error string would be
helpful, as attached.

(I'm also wondering why the second case is generic ERRCODE_SYNTAX_ERROR
and not ERRCODE_GENERATED_ALWAYS.  Didn't change it here, though.)

            regards, tom lane

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 839583f834..00a48686c4 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -861,7 +861,7 @@ rewriteTargetListIU(List *targetList,
                     if (!apply_default)
                         ereport(ERROR,
                                 (errcode(ERRCODE_GENERATED_ALWAYS),
-                                 errmsg("cannot insert into column \"%s\"",
+                                 errmsg("cannot insert a non-DEFAULT value into column \"%s\"",
                                         NameStr(att_tup->attname)),
                                  errdetail("Column \"%s\" is an identity column defined as GENERATED ALWAYS.",
                                            NameStr(att_tup->attname)),
@@ -900,7 +900,7 @@ rewriteTargetListIU(List *targetList,
                 if (!apply_default)
                     ereport(ERROR,
                             (errcode(ERRCODE_SYNTAX_ERROR),
-                             errmsg("cannot insert into column \"%s\"",
+                             errmsg("cannot insert a non-DEFAULT value into column \"%s\"",
                                     NameStr(att_tup->attname)),
                              errdetail("Column \"%s\" is a generated column.",
                                        NameStr(att_tup->attname))));
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index 559fafa09e..ca721d38bf 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -93,16 +93,16 @@ LINE 1: ...E gtest_err_8 (a int PRIMARY KEY, b int GENERATED BY DEFAULT...
 INSERT INTO gtest1 VALUES (1);
 INSERT INTO gtest1 VALUES (2, DEFAULT);  -- ok
 INSERT INTO gtest1 VALUES (3, 33);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1 VALUES (3, 33), (4, 44);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1 VALUES (3, DEFAULT), (4, 44);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1 VALUES (3, 33), (4, DEFAULT);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1 VALUES (3, DEFAULT), (4, DEFAULT);  -- ok
 SELECT * FROM gtest1 ORDER BY a;
@@ -193,25 +193,25 @@ SELECT * FROM gtest1v;
 (1 row)

 INSERT INTO gtest1v VALUES (4, 8);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1v VALUES (5, DEFAULT);  -- ok
 INSERT INTO gtest1v VALUES (6, 66), (7, 77);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1v VALUES (6, DEFAULT), (7, 77);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1v VALUES (6, 66), (7, DEFAULT);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1v VALUES (6, DEFAULT), (7, DEFAULT);  -- ok
 ALTER VIEW gtest1v ALTER COLUMN b SET DEFAULT 100;
 INSERT INTO gtest1v VALUES (8, DEFAULT);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 INSERT INTO gtest1v VALUES (8, DEFAULT), (9, DEFAULT);  -- error
-ERROR:  cannot insert into column "b"
+ERROR:  cannot insert a non-DEFAULT value into column "b"
 DETAIL:  Column "b" is a generated column.
 SELECT * FROM gtest1v;
  a | b
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 7e36b36ecd..fbca0333a2 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -107,20 +107,20 @@ SELECT * FROM itest4;
 -- VALUES RTEs
 CREATE TABLE itest5 (a int generated always as identity, b text);
 INSERT INTO itest5 VALUES (1, 'a');  -- error
-ERROR:  cannot insert into column "a"
+ERROR:  cannot insert a non-DEFAULT value into column "a"
 DETAIL:  Column "a" is an identity column defined as GENERATED ALWAYS.
 HINT:  Use OVERRIDING SYSTEM VALUE to override.
 INSERT INTO itest5 VALUES (DEFAULT, 'a');  -- ok
 INSERT INTO itest5 VALUES (2, 'b'), (3, 'c');  -- error
-ERROR:  cannot insert into column "a"
+ERROR:  cannot insert a non-DEFAULT value into column "a"
 DETAIL:  Column "a" is an identity column defined as GENERATED ALWAYS.
 HINT:  Use OVERRIDING SYSTEM VALUE to override.
 INSERT INTO itest5 VALUES (DEFAULT, 'b'), (3, 'c');  -- error
-ERROR:  cannot insert into column "a"
+ERROR:  cannot insert a non-DEFAULT value into column "a"
 DETAIL:  Column "a" is an identity column defined as GENERATED ALWAYS.
 HINT:  Use OVERRIDING SYSTEM VALUE to override.
 INSERT INTO itest5 VALUES (2, 'b'), (DEFAULT, 'c');  -- error
-ERROR:  cannot insert into column "a"
+ERROR:  cannot insert a non-DEFAULT value into column "a"
 DETAIL:  Column "a" is an identity column defined as GENERATED ALWAYS.
 HINT:  Use OVERRIDING SYSTEM VALUE to override.
 INSERT INTO itest5 VALUES (DEFAULT, 'b'), (DEFAULT, 'c');  -- ok
@@ -197,7 +197,7 @@ SELECT * FROM itest1;
 -- GENERATED ALWAYS
 -- This is an error:
 INSERT INTO itest2 VALUES (10, 'xyz');
-ERROR:  cannot insert into column "a"
+ERROR:  cannot insert a non-DEFAULT value into column "a"
 DETAIL:  Column "a" is an identity column defined as GENERATED ALWAYS.
 HINT:  Use OVERRIDING SYSTEM VALUE to override.
 -- This inserts the row as presented:
@@ -313,7 +313,7 @@ SELECT * FROM itestv10;
 (4 rows)

 INSERT INTO itestv11 VALUES (10, 'xyz');
-ERROR:  cannot insert into column "a"
+ERROR:  cannot insert a non-DEFAULT value into column "a"
 DETAIL:  Column "a" is an identity column defined as GENERATED ALWAYS.
 HINT:  Use OVERRIDING SYSTEM VALUE to override.
 INSERT INTO itestv11 OVERRIDING SYSTEM VALUE VALUES (11, 'xyz');
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 6a977006ef..24905332b1 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -1475,10 +1475,10 @@ INSERT INTO rw_view1 (id) VALUES (2);
 INSERT INTO base_tbl (id, idplus1) VALUES (3, DEFAULT);
 INSERT INTO rw_view1 (id, idplus1) VALUES (4, DEFAULT);
 INSERT INTO base_tbl (id, idplus1) VALUES (5, 6);  -- error
-ERROR:  cannot insert into column "idplus1"
+ERROR:  cannot insert a non-DEFAULT value into column "idplus1"
 DETAIL:  Column "idplus1" is a generated column.
 INSERT INTO rw_view1 (id, idplus1) VALUES (6, 7);  -- error
-ERROR:  cannot insert into column "idplus1"
+ERROR:  cannot insert a non-DEFAULT value into column "idplus1"
 DETAIL:  Column "idplus1" is a generated column.
 SELECT * FROM base_tbl;
  id | idplus1