Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9b9c5f2

Browse files
committed
Clarify behavior of adding and altering a column in same ALTER command.
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
1 parent affdde2 commit 9b9c5f2

File tree

4 files changed

+59
-12
lines changed

4 files changed

+59
-12
lines changed

doc/src/sgml/ref/alter_table.sgml

+35-4
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,11 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
203203
<term><literal>SET</literal>/<literal>DROP DEFAULT</literal></term>
204204
<listitem>
205205
<para>
206-
These forms set or remove the default value for a column.
207-
Default values only apply in subsequent <command>INSERT</command>
208-
or <command>UPDATE</command> commands; they do not cause rows already in the
209-
table to change.
206+
These forms set or remove the default value for a column (where
207+
removal is equivalent to setting the default value to NULL). The new
208+
default value will only apply in subsequent <command>INSERT</command>
209+
or <command>UPDATE</command> commands; it does not cause rows already
210+
in the table to change.
210211
</para>
211212
</listitem>
212213
</varlistentry>
@@ -268,6 +269,10 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
268269
These forms change whether a column is an identity column or change the
269270
generation attribute of an existing identity column.
270271
See <xref linkend="sql-createtable"/> for details.
272+
Like <literal>SET DEFAULT</literal>, these forms only affect the
273+
behavior of subsequent <command>INSERT</command>
274+
and <command>UPDATE</command> commands; they do not cause rows
275+
already in the table to change.
271276
</para>
272277

273278
<para>
@@ -1370,6 +1375,32 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
13701375
<programlisting>
13711376
ALTER TABLE distributors ADD COLUMN address varchar(30);
13721377
</programlisting>
1378+
That will cause all existing rows in the table to be filled with null
1379+
values for the new column.
1380+
</para>
1381+
1382+
<para>
1383+
To add a column with a non-null default:
1384+
<programlisting>
1385+
ALTER TABLE measurements
1386+
ADD COLUMN mtime timestamp with time zone DEFAULT now();
1387+
</programlisting>
1388+
Existing rows will be filled with the current time as the value of the
1389+
new column, and then new rows will receive the time of their insertion.
1390+
</para>
1391+
1392+
<para>
1393+
To add a column and fill it with a value different from the default to
1394+
be used later:
1395+
<programlisting>
1396+
ALTER TABLE transactions
1397+
ADD COLUMN status varchar(30) DEFAULT 'old',
1398+
ALTER COLUMN status SET default 'current';
1399+
</programlisting>
1400+
Existing rows will be filled with <literal>old</literal>, but then
1401+
the default for subsequent commands will be <literal>current</literal>.
1402+
The effects are the same as if the two sub-commands had been issued
1403+
in separate <command>ALTER TABLE</command> commands.
13731404
</para>
13741405

13751406
<para>

src/backend/commands/tablecmds.c

+12-8
Original file line numberDiff line numberDiff line change
@@ -6126,14 +6126,18 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
61266126
* returned by AddRelationNewConstraints, so that the right thing happens
61276127
* when a datatype's default applies.
61286128
*
6129-
* We skip this step completely for views and foreign tables. For a view,
6130-
* we can only get here from CREATE OR REPLACE VIEW, which historically
6131-
* doesn't set up defaults, not even for domain-typed columns. And in any
6132-
* case we mustn't invoke Phase 3 on a view or foreign table, since they
6133-
* have no storage.
6134-
*/
6135-
if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE
6136-
&& relkind != RELKIND_FOREIGN_TABLE && attribute.attnum > 0)
6129+
* Note: it might seem that this should happen at the end of Phase 2, so
6130+
* that the effects of subsequent subcommands can be taken into account.
6131+
* It's intentional that we do it now, though. The new column should be
6132+
* filled according to what is said in the ADD COLUMN subcommand, so that
6133+
* the effects are the same as if this subcommand had been run by itself
6134+
* and the later subcommands had been issued in new ALTER TABLE commands.
6135+
*
6136+
* We can skip this entirely for relations without storage, since Phase 3
6137+
* is certainly not going to touch them. System attributes don't have
6138+
* interesting defaults, either.
6139+
*/
6140+
if (RELKIND_HAS_STORAGE(relkind) && attribute.attnum > 0)
61376141
{
61386142
/*
61396143
* For an identity column, we can't use build_column_default(),

src/test/regress/expected/identity.out

+6
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,12 @@ ALTER TABLE itest8
409409
ALTER COLUMN f5 DROP NOT NULL,
410410
ALTER COLUMN f5 SET DATA TYPE bigint;
411411
INSERT INTO itest8 VALUES(0), (1);
412+
-- This does not work when the table isn't empty. That's intentional,
413+
-- since ADD GENERATED should only affect later insertions:
414+
ALTER TABLE itest8
415+
ADD COLUMN f22 int NOT NULL,
416+
ALTER COLUMN f22 ADD GENERATED ALWAYS AS IDENTITY;
417+
ERROR: column "f22" contains null values
412418
TABLE itest8;
413419
f1 | f2 | f3 | f4 | f5
414420
----+----+----+----+----

src/test/regress/sql/identity.sql

+6
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,12 @@ ALTER TABLE itest8
269269

270270
INSERT INTO itest8 VALUES(0), (1);
271271

272+
-- This does not work when the table isn't empty. That's intentional,
273+
-- since ADD GENERATED should only affect later insertions:
274+
ALTER TABLE itest8
275+
ADD COLUMN f22 int NOT NULL,
276+
ALTER COLUMN f22 ADD GENERATED ALWAYS AS IDENTITY;
277+
272278
TABLE itest8;
273279
\d+ itest8
274280
\d itest8_f2_seq

0 commit comments

Comments
 (0)