Clarify behavior of adding and altering a column in same ALTER command.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 21 Jan 2020 21:17:21 +0000 (16:17 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 21 Jan 2020 21:17:21 +0000 (16:17 -0500)
The behavior of something like

ALTER TABLE transactions
  ADD COLUMN status varchar(30) DEFAULT 'old',
  ALTER COLUMN status SET default 'current';

is to fill existing table rows with 'old', not 'current'.  That's
intentional and desirable for a couple of reasons:

* It makes the behavior the same whether you merge the sub-commands
into one ALTER command or give them separately;

* If we applied the new default while filling the table, there would
be no way to get the existing behavior in one SQL command.

The same reasoning applies in cases that add a column and then
manipulate its GENERATED/IDENTITY status in a second sub-command,
since the generation expression is really just a kind of default.
However, that wasn't very obvious (at least not to me; earlier in
the referenced discussion thread I'd thought it was a bug to be
fixed).  And it certainly wasn't documented.

Hence, add documentation, code comments, and a test case to clarify
that this behavior is all intentional.

In passing, adjust ATExecAddColumn's defaults-related relkind check
so that it matches up exactly with ATRewriteTables, instead of being
effectively (though not literally) the negated inverse condition.
The reasoning can be explained a lot more concisely that way, too
(not to mention that the comment now matches the code, which it
did not before).

Discussion: https://postgr.es/m/10365.1558909428@sss.pgh.pa.us

doc/src/sgml/ref/alter_table.sgml
src/backend/commands/tablecmds.c
src/test/regress/expected/identity.out
src/test/regress/sql/identity.sql

index 4bf449587ccb2eae8c7dbebc130c18c43ef36c71..1c222d63e0b4529683b6ec8100ea0fc3c905a651 100644 (file)
@@ -203,10 +203,11 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
     <term><literal>SET</literal>/<literal>DROP DEFAULT</literal></term>
     <listitem>
      <para>
-      These forms set or remove the default value for a column.
-      Default values only apply in subsequent <command>INSERT</command>
-      or <command>UPDATE</command> commands; they do not cause rows already in the
-      table to change.
+      These forms set or remove the default value for a column (where
+      removal is equivalent to setting the default value to NULL).  The new
+      default value will only apply in subsequent <command>INSERT</command>
+      or <command>UPDATE</command> commands; it does not cause rows already
+      in the table to change.
      </para>
     </listitem>
    </varlistentry>
@@ -268,6 +269,10 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       These forms change whether a column is an identity column or change the
       generation attribute of an existing identity column.
       See <xref linkend="sql-createtable"/> for details.
+      Like <literal>SET DEFAULT</literal>, these forms only affect the
+      behavior of subsequent <command>INSERT</command>
+      and <command>UPDATE</command> commands; they do not cause rows
+      already in the table to change.
      </para>
 
      <para>
@@ -1370,6 +1375,32 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
 <programlisting>
 ALTER TABLE distributors ADD COLUMN address varchar(30);
 </programlisting>
+   That will cause all existing rows in the table to be filled with null
+   values for the new column.
+  </para>
+
+  <para>
+   To add a column with a non-null default:
+<programlisting>
+ALTER TABLE measurements
+  ADD COLUMN mtime timestamp with time zone DEFAULT now();
+</programlisting>
+   Existing rows will be filled with the current time as the value of the
+   new column, and then new rows will receive the time of their insertion.
+  </para>
+
+  <para>
+   To add a column and fill it with a value different from the default to
+   be used later:
+<programlisting>
+ALTER TABLE transactions
+  ADD COLUMN status varchar(30) DEFAULT 'old',
+  ALTER COLUMN status SET default 'current';
+</programlisting>
+   Existing rows will be filled with <literal>old</literal>, but then
+   the default for subsequent commands will be <literal>current</literal>.
+   The effects are the same as if the two sub-commands had been issued
+   in separate <command>ALTER TABLE</command> commands.
   </para>
 
   <para>
index 30b72b62971be41e62c1e99d0c16e1358d14cf77..faf0db99e2a8256afd32ffa6329965d75c98144c 100644 (file)
@@ -6126,14 +6126,18 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
     * returned by AddRelationNewConstraints, so that the right thing happens
     * when a datatype's default applies.
     *
-    * We skip this step completely for views and foreign tables.  For a view,
-    * we can only get here from CREATE OR REPLACE VIEW, which historically
-    * doesn't set up defaults, not even for domain-typed columns.  And in any
-    * case we mustn't invoke Phase 3 on a view or foreign table, since they
-    * have no storage.
-    */
-   if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE
-       && relkind != RELKIND_FOREIGN_TABLE && attribute.attnum > 0)
+    * Note: it might seem that this should happen at the end of Phase 2, so
+    * that the effects of subsequent subcommands can be taken into account.
+    * It's intentional that we do it now, though.  The new column should be
+    * filled according to what is said in the ADD COLUMN subcommand, so that
+    * the effects are the same as if this subcommand had been run by itself
+    * and the later subcommands had been issued in new ALTER TABLE commands.
+    *
+    * We can skip this entirely for relations without storage, since Phase 3
+    * is certainly not going to touch them.  System attributes don't have
+    * interesting defaults, either.
+    */
+   if (RELKIND_HAS_STORAGE(relkind) && attribute.attnum > 0)
    {
        /*
         * For an identity column, we can't use build_column_default(),
index 7cf4696ec9536310ea3f8891f47078244a665052..1a614b85f995277117c949e771a3122de918ce1e 100644 (file)
@@ -409,6 +409,12 @@ ALTER TABLE itest8
   ALTER COLUMN f5 DROP NOT NULL,
   ALTER COLUMN f5 SET DATA TYPE bigint;
 INSERT INTO itest8 VALUES(0), (1);
+-- This does not work when the table isn't empty.  That's intentional,
+-- since ADD GENERATED should only affect later insertions:
+ALTER TABLE itest8
+  ADD COLUMN f22 int NOT NULL,
+  ALTER COLUMN f22 ADD GENERATED ALWAYS AS IDENTITY;
+ERROR:  column "f22" contains null values
 TABLE itest8;
  f1 | f2 | f3 | f4 | f5 
 ----+----+----+----+----
index 685607c90c147fd1ec1e18b4b8767a24f9b87d68..b4cdd21bdd408f3241d72754a5c101a3dc0cc671 100644 (file)
@@ -269,6 +269,12 @@ ALTER TABLE itest8
 
 INSERT INTO itest8 VALUES(0), (1);
 
+-- This does not work when the table isn't empty.  That's intentional,
+-- since ADD GENERATED should only affect later insertions:
+ALTER TABLE itest8
+  ADD COLUMN f22 int NOT NULL,
+  ALTER COLUMN f22 ADD GENERATED ALWAYS AS IDENTITY;
+
 TABLE itest8;
 \d+ itest8
 \d itest8_f2_seq