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

Commit f3dae2a

Browse files
jeff-davisjeff-davis
committed
Do not use in-place updates for statistics import.
The use of in-place updates was originally there to follow the precedent of ANALYZE and to reduce the potential for bloat on pg_class. Per discussion, it's not worth the risks. Reported-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/cpdanvzykcb5o64rmapkx6n5gjypoce3y52hff7ocxupgpbxu4@53jmlyvukijo
1 parent 3ce3575 commit f3dae2a

File tree

3 files changed

+47
-177
lines changed

3 files changed

+47
-177
lines changed

‎src/backend/statistics/relation_stats.c

+47-91
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,13 @@ static struct StatsArgInfo relarginfo[] =
4848
[NUM_RELATION_STATS_ARGS] = {0}
4949
};
5050

51-
static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel,
52-
bool inplace);
51+
static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel);
5352

5453
/*
5554
* Internal function for modifying statistics for a relation.
5655
*/
5756
static bool
58-
relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
57+
relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
5958
{
6059
bool result = true;
6160
Oid reloid;
@@ -66,6 +65,12 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
6665
bool update_reltuples = false;
6766
BlockNumber relallvisible = 0;
6867
bool update_relallvisible = false;
68+
HeapTuple ctup;
69+
Form_pg_class pgcform;
70+
int replaces[3] = {0};
71+
Datum values[3] = {0};
72+
bool nulls[3] = {0};
73+
int nreplaces = 0;
6974

7075
if (!PG_ARGISNULL(RELPAGES_ARG))
7176
{
@@ -110,101 +115,52 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
110115
*/
111116
crel = table_open(RelationRelationId, RowExclusiveLock);
112117

113-
if (inplace)
118+
ctup = SearchSysCache1(RELOID, ObjectIdGetDatum(reloid));
119+
if (!HeapTupleIsValid(ctup))
114120
{
115-
HeapTuple ctup = NULL;
116-
ScanKeyData key[1];
117-
Form_pg_class pgcform;
118-
void *inplace_state = NULL;
119-
bool dirty = false;
120-
121-
ScanKeyInit(&key[0], Anum_pg_class_oid, BTEqualStrategyNumber, F_OIDEQ,
122-
ObjectIdGetDatum(reloid));
123-
systable_inplace_update_begin(crel, ClassOidIndexId, true, NULL, 1, key,
124-
&ctup, &inplace_state);
125-
if (!HeapTupleIsValid(ctup))
126-
elog(ERROR, "pg_class entry for relid %u vanished while updating statistics",
127-
reloid);
128-
pgcform = (Form_pg_class) GETSTRUCT(ctup);
129-
130-
if (update_relpages && pgcform->relpages != relpages)
131-
{
132-
pgcform->relpages = relpages;
133-
dirty = true;
134-
}
135-
if (update_reltuples && pgcform->reltuples != reltuples)
136-
{
137-
pgcform->reltuples = reltuples;
138-
dirty = true;
139-
}
140-
if (update_relallvisible && pgcform->relallvisible != relallvisible)
141-
{
142-
pgcform->relallvisible = relallvisible;
143-
dirty = true;
144-
}
145-
146-
if (dirty)
147-
systable_inplace_update_finish(inplace_state, ctup);
148-
else
149-
systable_inplace_update_cancel(inplace_state);
150-
151-
heap_freetuple(ctup);
121+
ereport(elevel,
122+
(errcode(ERRCODE_OBJECT_IN_USE),
123+
errmsg("pg_class entry for relid %u not found", reloid)));
124+
table_close(crel, RowExclusiveLock);
125+
return false;
152126
}
153-
else
154-
{
155-
TupleDesc tupdesc = RelationGetDescr(crel);
156-
HeapTuple ctup;
157-
Form_pg_class pgcform;
158-
int replaces[3] = {0};
159-
Datum values[3] = {0};
160-
bool nulls[3] = {0};
161-
int nreplaces = 0;
162-
163-
ctup = SearchSysCache1(RELOID, ObjectIdGetDatum(reloid));
164-
if (!HeapTupleIsValid(ctup))
165-
{
166-
ereport(elevel,
167-
(errcode(ERRCODE_OBJECT_IN_USE),
168-
errmsg("pg_class entry for relid %u not found", reloid)));
169-
table_close(crel, RowExclusiveLock);
170-
return false;
171-
}
172-
pgcform = (Form_pg_class) GETSTRUCT(ctup);
173127

174-
if (update_relpages && relpages != pgcform->relpages)
175-
{
176-
replaces[nreplaces] = Anum_pg_class_relpages;
177-
values[nreplaces] = UInt32GetDatum(relpages);
178-
nreplaces++;
179-
}
128+
pgcform = (Form_pg_class) GETSTRUCT(ctup);
180129

181-
if (update_reltuples && reltuples != pgcform->reltuples)
182-
{
183-
replaces[nreplaces] = Anum_pg_class_reltuples;
184-
values[nreplaces] = Float4GetDatum(reltuples);
185-
nreplaces++;
186-
}
130+
if (update_relpages && relpages != pgcform->relpages)
131+
{
132+
replaces[nreplaces] = Anum_pg_class_relpages;
133+
values[nreplaces] = UInt32GetDatum(relpages);
134+
nreplaces++;
135+
}
187136

188-
if (update_relallvisible && relallvisible != pgcform->relallvisible)
189-
{
190-
replaces[nreplaces] = Anum_pg_class_relallvisible;
191-
values[nreplaces] = UInt32GetDatum(relallvisible);
192-
nreplaces++;
193-
}
137+
if (update_reltuples && reltuples != pgcform->reltuples)
138+
{
139+
replaces[nreplaces] = Anum_pg_class_reltuples;
140+
values[nreplaces] = Float4GetDatum(reltuples);
141+
nreplaces++;
142+
}
194143

195-
if (nreplaces > 0)
196-
{
197-
HeapTuple newtup;
144+
if (update_relallvisible && relallvisible != pgcform->relallvisible)
145+
{
146+
replaces[nreplaces] = Anum_pg_class_relallvisible;
147+
values[nreplaces] = UInt32GetDatum(relallvisible);
148+
nreplaces++;
149+
}
198150

199-
newtup = heap_modify_tuple_by_cols(ctup, tupdesc, nreplaces,
200-
replaces, values, nulls);
201-
CatalogTupleUpdate(crel, &newtup->t_self, newtup);
202-
heap_freetuple(newtup);
203-
}
151+
if (nreplaces > 0)
152+
{
153+
TupleDesc tupdesc = RelationGetDescr(crel);
154+
HeapTuple newtup;
204155

205-
ReleaseSysCache(ctup);
156+
newtup = heap_modify_tuple_by_cols(ctup, tupdesc, nreplaces,
157+
replaces, values, nulls);
158+
CatalogTupleUpdate(crel, &newtup->t_self, newtup);
159+
heap_freetuple(newtup);
206160
}
207161

162+
ReleaseSysCache(ctup);
163+
208164
/* release the lock, consistent with vac_update_relstats() */
209165
table_close(crel, RowExclusiveLock);
210166

@@ -219,7 +175,7 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
219175
Datum
220176
pg_set_relation_stats(PG_FUNCTION_ARGS)
221177
{
222-
relation_statistics_update(fcinfo, ERROR, false);
178+
relation_statistics_update(fcinfo, ERROR);
223179
PG_RETURN_VOID();
224180
}
225181

@@ -243,7 +199,7 @@ pg_clear_relation_stats(PG_FUNCTION_ARGS)
243199
newfcinfo->args[3].value = UInt32GetDatum(0);
244200
newfcinfo->args[3].isnull = false;
245201

246-
relation_statistics_update(newfcinfo, ERROR, false);
202+
relation_statistics_update(newfcinfo, ERROR);
247203
PG_RETURN_VOID();
248204
}
249205

@@ -261,7 +217,7 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS)
261217
relarginfo, WARNING))
262218
result = false;
263219

264-
if (!relation_statistics_update(positional_fcinfo, WARNING, true))
220+
if (!relation_statistics_update(positional_fcinfo, WARNING))
265221
result = false;
266222

267223
PG_RETURN_BOOL(result);

‎src/test/regress/expected/stats_import.out

-53
Original file line numberDiff line numberDiff line change
@@ -143,39 +143,6 @@ WHERE oid = 'stats_import.test'::regclass;
143143
18 | 401 | 5
144144
(1 row)
145145

146-
-- test MVCC behavior: changes do not persist after abort (in contrast
147-
-- to pg_restore_relation_stats(), which uses in-place updates).
148-
BEGIN;
149-
SELECT
150-
pg_catalog.pg_set_relation_stats(
151-
relation => 'stats_import.test'::regclass,
152-
relpages => NULL::integer,
153-
reltuples => 4000.0::real,
154-
relallvisible => 4::integer);
155-
pg_set_relation_stats
156-
-----------------------
157-
158-
(1 row)
159-
160-
ABORT;
161-
SELECT relpages, reltuples, relallvisible
162-
FROM pg_class
163-
WHERE oid = 'stats_import.test'::regclass;
164-
relpages | reltuples | relallvisible
165-
----------+-----------+---------------
166-
18 | 401 | 5
167-
(1 row)
168-
169-
BEGIN;
170-
SELECT
171-
pg_catalog.pg_clear_relation_stats(
172-
'stats_import.test'::regclass);
173-
pg_clear_relation_stats
174-
-------------------------
175-
176-
(1 row)
177-
178-
ABORT;
179146
SELECT relpages, reltuples, relallvisible
180147
FROM pg_class
181148
WHERE oid = 'stats_import.test'::regclass;
@@ -836,25 +803,6 @@ WHERE oid = 'stats_import.test'::regclass;
836803
(1 row)
837804

838805
-- ok: just relpages
839-
SELECT pg_restore_relation_stats(
840-
'relation', 'stats_import.test'::regclass,
841-
'version', 150000::integer,
842-
'relpages', '15'::integer);
843-
pg_restore_relation_stats
844-
---------------------------
845-
t
846-
(1 row)
847-
848-
SELECT relpages, reltuples, relallvisible
849-
FROM pg_class
850-
WHERE oid = 'stats_import.test'::regclass;
851-
relpages | reltuples | relallvisible
852-
----------+-----------+---------------
853-
15 | 400 | 4
854-
(1 row)
855-
856-
-- test non-MVCC behavior: new value should persist after abort
857-
BEGIN;
858806
SELECT pg_restore_relation_stats(
859807
'relation', 'stats_import.test'::regclass,
860808
'version', 150000::integer,
@@ -864,7 +812,6 @@ SELECT pg_restore_relation_stats(
864812
t
865813
(1 row)
866814

867-
ABORT;
868815
SELECT relpages, reltuples, relallvisible
869816
FROM pg_class
870817
WHERE oid = 'stats_import.test'::regclass;

‎src/test/regress/sql/stats_import.sql

-33
Original file line numberDiff line numberDiff line change
@@ -100,27 +100,6 @@ SELECT relpages, reltuples, relallvisible
100100
FROM pg_class
101101
WHERE oid = 'stats_import.test'::regclass;
102102

103-
-- test MVCC behavior: changes do not persist after abort (in contrast
104-
-- to pg_restore_relation_stats(), which uses in-place updates).
105-
BEGIN;
106-
SELECT
107-
pg_catalog.pg_set_relation_stats(
108-
relation => 'stats_import.test'::regclass,
109-
relpages => NULL::integer,
110-
reltuples => 4000.0::real,
111-
relallvisible => 4::integer);
112-
ABORT;
113-
114-
SELECT relpages, reltuples, relallvisible
115-
FROM pg_class
116-
WHERE oid = 'stats_import.test'::regclass;
117-
118-
BEGIN;
119-
SELECT
120-
pg_catalog.pg_clear_relation_stats(
121-
'stats_import.test'::regclass);
122-
ABORT;
123-
124103
SELECT relpages, reltuples, relallvisible
125104
FROM pg_class
126105
WHERE oid = 'stats_import.test'::regclass;
@@ -649,22 +628,10 @@ FROM pg_class
649628
WHERE oid = 'stats_import.test'::regclass;
650629

651630
-- ok: just relpages
652-
SELECT pg_restore_relation_stats(
653-
'relation', 'stats_import.test'::regclass,
654-
'version', 150000::integer,
655-
'relpages', '15'::integer);
656-
657-
SELECT relpages, reltuples, relallvisible
658-
FROM pg_class
659-
WHERE oid = 'stats_import.test'::regclass;
660-
661-
-- test non-MVCC behavior: new value should persist after abort
662-
BEGIN;
663631
SELECT pg_restore_relation_stats(
664632
'relation', 'stats_import.test'::regclass,
665633
'version', 150000::integer,
666634
'relpages', '16'::integer);
667-
ABORT;
668635

669636
SELECT relpages, reltuples, relallvisible
670637
FROM pg_class

0 commit comments

Comments
 (0)