Skip to content

Commit f9b5b41

Browse files
tglsfdctglsfdc
committed
Code review for ON COMMIT patch. Make the actual on-commit action happen
before commit, not after :-( --- the original coding is not only unsafe if an error occurs while it's processing, but it generates an invalid sequence of WAL entries. Resurrect 7.2 logic for deleting items when no longer needed. Use an enum instead of random macros. Editorialize on names used for routines and constants. Teach backend/nodes routines about new field in CreateTable struct. Add a regression test.
1 parent 1b342df commit f9b5b41

File tree

21 files changed

+344
-316
lines changed

21 files changed

+344
-316
lines changed

‎doc/src/sgml/ref/create_table.sgml

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_table.sgml,v 1.57 2002/11/09 23:56:38 momjian Exp $
2+
$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_table.sgml,v 1.58 2002/11/11 22:19:20 tgl Exp $
33
PostgreSQL documentation
44
-->
55

@@ -21,7 +21,8 @@ CREATE [ [ LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PARAMETER">t
2121
| <replaceable>table_constraint</replaceable> } [, ... ]
2222
)
2323
[ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
24-
[ WITH OIDS | WITHOUT OIDS ] [ON COMMIT { PRESERVE ROWS | DELETE ROWS | DROP } ]
24+
[ WITH OIDS | WITHOUT OIDS ]
25+
[ ON COMMIT { PRESERVE ROWS | DELETE ROWS | DROP } ]
2526

2627
where <replaceable class="PARAMETER">column_constraint</replaceable> is:
2728

@@ -107,8 +108,8 @@ and <replaceable class="PARAMETER">table_constraint</replaceable> is:
107108
<para>
108109
If specified, the table is created as a temporary table.
109110
Temporary tables are automatically dropped at the end of a
110-
session or optionally at the end of the current transaction
111-
(See ON COMMIT below). Existing permanent tables with the same
111+
session, or optionally at the end of the current transaction
112+
(see ON COMMIT below). Existing permanent tables with the same
112113
name are not visible to the current session while the temporary
113114
table exists, unless they are referenced with schema-qualified
114115
names. Any indexes created on a temporary table are automatically
@@ -493,22 +494,17 @@ and <replaceable class="PARAMETER">table_constraint</replaceable> is:
493494
<term><literal>ON COMMIT</literal></term>
494495
<listitem>
495496
<para>
496-
The behaviour of temporary tables at the end of a transaction
497+
The behavior of temporary tables at the end of a transaction
497498
block can be controlled using <literal>ON COMMIT</literal>.
498-
The table will exhibit the same behavior at the end of
499-
transaction blocks for the duration of the session unless
500-
ON COMMIT DROP is specified or the temporary table is dropped.
501-
</para>
502-
<para>
503-
The three parameters to ON COMMIT are:
499+
The three options are:
504500

505501
<variablelist>
506502
<varlistentry>
507503
<term><literal>PRESERVE ROWS</literal></term>
508504
<listitem>
509505
<para>
510-
The rows in the temporary table will persist after the
511-
transaction block.
506+
No special action is taken at the ends of transactions.
507+
This is the default behavior.
512508
</para>
513509
</listitem>
514510
</varlistentry>
@@ -517,8 +513,9 @@ and <replaceable class="PARAMETER">table_constraint</replaceable> is:
517513
<term><literal>DELETE ROWS</literal></term>
518514
<listitem>
519515
<para>
520-
All rows in the temporary table will be deleted at the
521-
end of the transaction block.
516+
All rows in the temporary table will be deleted at the
517+
end of each transaction block. Essentially, an automatic
518+
<xref linkend="sql-truncate"> is done at each commit.
522519
</para>
523520
</listitem>
524521
</varlistentry>
@@ -527,7 +524,8 @@ and <replaceable class="PARAMETER">table_constraint</replaceable> is:
527524
<term><literal>DROP</literal></term>
528525
<listitem>
529526
<para>
530-
The temporary table will be dropped at the end of the transaction.
527+
The temporary table will be dropped at the end of the current
528+
transaction block.
531529
</para>
532530
</listitem>
533531
</varlistentry>

‎src/backend/access/transam/xact.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.136 2002/11/09 23:56:38 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.137 2002/11/11 22:19:20 tgl Exp $
1212
*
1313
* NOTES
1414
* Transaction aborts can now occur two ways:
@@ -951,6 +951,12 @@ CommitTransaction(void)
951951
*/
952952
DeferredTriggerEndXact();
953953

954+
/*
955+
* Similarly, let ON COMMIT management do its thing before we start
956+
* to commit.
957+
*/
958+
PreCommit_on_commit_actions();
959+
954960
/* Prevent cancel/die interrupt while cleaning up */
955961
HOLD_INTERRUPTS();
956962

@@ -1027,7 +1033,7 @@ CommitTransaction(void)
10271033
AtEOXact_hash();
10281034
AtEOXact_nbtree();
10291035
AtEOXact_rtree();
1030-
AtEOXact_temp_relations(true,s->blockState);
1036+
AtEOXact_on_commit_actions(true);
10311037
AtEOXact_Namespace(true);
10321038
AtEOXact_CatCache(true);
10331039
AtEOXact_Files();
@@ -1138,7 +1144,7 @@ AbortTransaction(void)
11381144
AtEOXact_hash();
11391145
AtEOXact_nbtree();
11401146
AtEOXact_rtree();
1141-
AtEOXact_temp_relations(false,s->blockState);
1147+
AtEOXact_on_commit_actions(false);
11421148
AtEOXact_Namespace(false);
11431149
AtEOXact_CatCache(false);
11441150
AtEOXact_Files();

‎src/backend/bootstrap/bootparse.y

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $Header: /cvsroot/pgsql/src/backend/bootstrap/bootparse.y,v 1.54 2002/11/09 23:56:38 momjian Exp $
12+
* $Header: /cvsroot/pgsql/src/backend/bootstrap/bootparse.y,v 1.55 2002/11/11 22:19:21 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -34,7 +34,6 @@
3434
#include "catalog/pg_class.h"
3535
#include "catalog/pg_namespace.h"
3636
#include "commands/defrem.h"
37-
#include "commands/tablecmds.h"
3837
#include "miscadmin.h"
3938
#include "nodes/makefuncs.h"
4039
#include "nodes/nodes.h"
@@ -198,7 +197,7 @@ Boot_CreateStmt:
198197
tupdesc,
199198
RELKIND_RELATION,
200199
$3,
201-
ATEOXACTNOOP,
200+
ONCOMMIT_NOOP,
202201
true);
203202
elog(DEBUG3, "relation created with oid %u", id);
204203
}

‎src/backend/catalog/heap.c

Lines changed: 20 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.233 2002/11/09 23:56:39 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.234 2002/11/11 22:19:21 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -31,14 +31,12 @@
3131

3232
#include "access/heapam.h"
3333
#include "access/genam.h"
34-
#include "access/xact.h"
3534
#include "catalog/catalog.h"
3635
#include "catalog/catname.h"
3736
#include "catalog/dependency.h"
3837
#include "catalog/heap.h"
3938
#include "catalog/index.h"
4039
#include "catalog/indexing.h"
41-
#include "catalog/namespace.h"
4240
#include "catalog/pg_attrdef.h"
4341
#include "catalog/pg_constraint.h"
4442
#include "catalog/pg_inherits.h"
@@ -673,14 +671,13 @@ AddNewRelationType(const char *typeName,
673671
* creates a new cataloged relation. see comments above.
674672
* --------------------------------
675673
*/
676-
677674
Oid
678675
heap_create_with_catalog(const char *relname,
679676
Oid relnamespace,
680677
TupleDesc tupdesc,
681678
char relkind,
682679
bool shared_relation,
683-
char ateoxact, /* Only used for temp relations */
680+
OnCommitAction oncommit,
684681
bool allow_system_table_mods)
685682
{
686683
Relation pg_class_desc;
@@ -722,25 +719,6 @@ heap_create_with_catalog(const char *relname,
722719
/* Assign an OID for the relation's tuple type */
723720
new_type_oid = newoid();
724721

725-
726-
/*
727-
* Add to temprels if we are a temp relation now that we have oid
728-
*/
729-
730-
if(isTempNamespace(relnamespace)) {
731-
TempTable *t;
732-
MemoryContext oldcxt;
733-
734-
oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
735-
t = (TempTable *) palloc(sizeof(TempTable));
736-
t->relid = new_rel_oid;
737-
t->ateoxact = ateoxact;
738-
t->tid = GetCurrentTransactionId();
739-
t->dead = false;
740-
reg_temp_rel(t);
741-
MemoryContextSwitchTo(oldcxt);
742-
}
743-
744722
/*
745723
* now create an entry in pg_class for the relation.
746724
*
@@ -804,6 +782,12 @@ heap_create_with_catalog(const char *relname,
804782
*/
805783
StoreConstraints(new_rel_desc, tupdesc);
806784

785+
/*
786+
* If there's a special on-commit action, remember it
787+
*/
788+
if (oncommit != ONCOMMIT_NOOP)
789+
register_on_commit_action(new_rel_oid, oncommit);
790+
807791
/*
808792
* ok, the relation has been cataloged, so close our relations and
809793
* return the oid of the newly created relation.
@@ -1164,21 +1148,18 @@ heap_drop_with_catalog(Oid rid)
11641148
*/
11651149
DeleteRelationTuple(RelationGetRelid(rel));
11661150

1151+
/*
1152+
* forget any ON COMMIT action for the rel
1153+
*/
1154+
remove_on_commit_action(rid);
1155+
11671156
/*
11681157
* unlink the relation's physical file and finish up.
11691158
*/
11701159
if (rel->rd_rel->relkind != RELKIND_VIEW &&
11711160
rel->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
11721161
smgrunlink(DEFAULT_SMGR, rel);
11731162

1174-
/*
1175-
* Keep temprels up to date so that we don't have ON COMMIT execution
1176-
* problems at the end of the next transaction block
1177-
*/
1178-
1179-
if(isTempNamespace(RelationGetNamespace(rel)))
1180-
rm_temp_rel(rid);
1181-
11821163
/*
11831164
* Close relcache entry, but *keep* AccessExclusiveLock on the
11841165
* relation until transaction commit. This ensures no one else will
@@ -1941,12 +1922,13 @@ RelationTruncateIndexes(Oid heapId)
19411922
*
19421923
* This routine is used to truncate the data from the
19431924
* storage manager of any data within the relation handed
1944-
* to this routine.
1925+
* to this routine. This is not transaction-safe!
19451926
*/
19461927
void
19471928
heap_truncate(Oid rid)
19481929
{
19491930
Relation rel;
1931+
Oid toastrelid;
19501932

19511933
/* Open relation for processing, and grab exclusive access on it. */
19521934
rel = heap_open(rid, AccessExclusiveLock);
@@ -1965,6 +1947,11 @@ heap_truncate(Oid rid)
19651947
/* If this relation has indexes, truncate the indexes too */
19661948
RelationTruncateIndexes(rid);
19671949

1950+
/* If it has a toast table, recursively truncate that too */
1951+
toastrelid = rel->rd_rel->reltoastrelid;
1952+
if (OidIsValid(toastrelid))
1953+
heap_truncate(toastrelid);
1954+
19681955
/*
19691956
* Close the relation, but keep exclusive lock on it until commit.
19701957
*/

‎src/backend/catalog/namespace.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* Portions Copyright (c) 1994, Regents of the University of California
1414
*
1515
* IDENTIFICATION
16-
* $Header: /cvsroot/pgsql/src/backend/catalog/namespace.c,v 1.39 2002/11/09 23:56:39 momjian Exp $
16+
* $Header: /cvsroot/pgsql/src/backend/catalog/namespace.c,v 1.40 2002/11/11 22:19:21 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -34,7 +34,6 @@
3434
#include "catalog/pg_proc.h"
3535
#include "catalog/pg_shadow.h"
3636
#include "catalog/pg_type.h"
37-
#include "commands/tablecmds.h"
3837
#include "lib/stringinfo.h"
3938
#include "miscadmin.h"
4039
#include "nodes/makefuncs.h"
@@ -1671,7 +1670,6 @@ RemoveTempRelationsCallback(void)
16711670

16721671
CommitTransactionCommand(true);
16731672
}
1674-
free_temp_rels();
16751673
}
16761674

16771675

‎src/backend/commands/cluster.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*
1313
* IDENTIFICATION
14-
* $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.92 2002/11/09 23:56:39 momjian Exp $
14+
* $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.93 2002/11/11 22:19:21 tgl Exp $
1515
*
1616
*-------------------------------------------------------------------------
1717
*/
@@ -25,7 +25,6 @@
2525
#include "catalog/index.h"
2626
#include "catalog/indexing.h"
2727
#include "catalog/catname.h"
28-
#include "catalog/namespace.h"
2928
#include "commands/cluster.h"
3029
#include "commands/tablecmds.h"
3130
#include "miscadmin.h"
@@ -204,7 +203,7 @@ make_new_heap(Oid OIDOldHeap, const char *NewName)
204203
tupdesc,
205204
OldHeap->rd_rel->relkind,
206205
OldHeap->rd_rel->relisshared,
207-
ATEOXACTNOOP,
206+
ONCOMMIT_NOOP,
208207
allowSystemTableMods);
209208

210209
/*

‎src/backend/commands/sequence.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/commands/sequence.c,v 1.89 2002/11/10 00:10:20 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/commands/sequence.c,v 1.90 2002/11/11 22:19:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -193,6 +193,7 @@ DefineSequence(CreateSeqStmt *seq)
193193
stmt->inhRelations = NIL;
194194
stmt->constraints = NIL;
195195
stmt->hasoids = false;
196+
stmt->oncommit = ONCOMMIT_NOOP;
196197

197198
seqoid = DefineRelation(stmt, RELKIND_SEQUENCE);
198199

0 commit comments

Comments
 (0)