Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4b34624

Browse files
committed
Code review for commands/statscmds.c.
Fix machine-dependent sorting of column numbers. (Odd behaviorwould only materialize for column numbers above 255, but that'scertainly legal.)Fix poor choice of SQLSTATE for some errors, and improve error messagewording. (Notably, "is not a scalar type" is a totally misleading wayto explain "does not have a default btree opclass".)Avoid taking AccessExclusiveLock on the associated relation during DROPSTATISTICS. That's neither necessary nor desirable, and it could easilyhave put us into situations where DROP fails (compare commit68ea2b7).Adjust/improve comments.David Rowley and Tom LaneDiscussion:https://postgr.es/m/CAKJS1f-GmCfPvBbAEaM5xoVOaYdVgVN1gicALSoYQ77z-+vLbw@mail.gmail.com
1 parentb182a4a commit4b34624

File tree

2 files changed

+38
-31
lines changed

2 files changed

+38
-31
lines changed

‎src/backend/commands/statscmds.c‎

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,15 @@
3131
#include"utils/typcache.h"
3232

3333

34-
/*used for sorting the attnums in CreateStatistics */
34+
/*qsort comparator for the attnums in CreateStatistics */
3535
staticint
3636
compare_int16(constvoid*a,constvoid*b)
3737
{
38-
returnmemcmp(a,b,sizeof(int16));
38+
intav=*(constint16*)a;
39+
intbv=*(constint16*)b;
40+
41+
/* this can't overflow if int is wider than int16 */
42+
return (av-bv);
3943
}
4044

4145
/*
@@ -44,8 +48,6 @@ compare_int16(const void *a, const void *b)
4448
ObjectAddress
4549
CreateStatistics(CreateStatsStmt*stmt)
4650
{
47-
inti;
48-
ListCell*l;
4951
int16attnums[STATS_MAX_DIMENSIONS];
5052
intnumcols=0;
5153
ObjectAddressaddress=InvalidObjectAddress;
@@ -68,6 +70,8 @@ CreateStatistics(CreateStatsStmt *stmt)
6870
boolbuild_ndistinct;
6971
boolbuild_dependencies;
7072
boolrequested_type= false;
73+
inti;
74+
ListCell*l;
7175

7276
Assert(IsA(stmt,CreateStatsStmt));
7377

@@ -76,10 +80,10 @@ CreateStatistics(CreateStatsStmt *stmt)
7680
namestrcpy(&stxname,namestr);
7781

7882
/*
79-
*If if_not_exists was given and the statistics alreadyexists, bail out.
83+
*Deal with the possibility that thenamedstatistics alreadyexist.
8084
*/
8185
if (SearchSysCacheExists2(STATEXTNAMENSP,
82-
PointerGetDatum(&stxname),
86+
NameGetDatum(&stxname),
8387
ObjectIdGetDatum(namespaceId)))
8488
{
8589
if (stmt->if_not_exists)
@@ -97,10 +101,11 @@ CreateStatistics(CreateStatsStmt *stmt)
97101
}
98102

99103
/*
100-
* CREATE STATISTICS will influence future execution plans but does
101-
*notinterfere with currently executing plans soitis safe to
104+
* CREATE STATISTICS will influence future execution plans but does not
105+
* interfere with currently executing plans. Soitshould be enough to
102106
* take only ShareUpdateExclusiveLock on relation, conflicting with
103-
* ANALYZE and other DDL that sets statistical information.
107+
* ANALYZE and other DDL that sets statistical information, but not with
108+
* normal queries.
104109
*/
105110
rel=relation_openrv(stmt->relation,ShareUpdateExclusiveLock);
106111
relid=RelationGetRelid(rel);
@@ -137,25 +142,26 @@ CreateStatistics(CreateStatsStmt *stmt)
137142
if (attForm->attnum<0)
138143
ereport(ERROR,
139144
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
140-
errmsg("statistic creation on system columns is not supported")));
145+
errmsg("statistics creation on system columns is not supported")));
141146

142147
/* Disallow data types without a less-than operator */
143148
type=lookup_type_cache(attForm->atttypid,TYPECACHE_LT_OPR);
144149
if (type->lt_opr==InvalidOid)
145150
ereport(ERROR,
146151
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
147-
errmsg("only scalar types can be used in extended statistics")));
152+
errmsg("column \"%s\" cannot be used in statistics because its type has no default btree operator class",
153+
attname)));
148154

149155
/* Make sure no more than STATS_MAX_DIMENSIONS columns are used */
150156
if (numcols >=STATS_MAX_DIMENSIONS)
151157
ereport(ERROR,
152158
(errcode(ERRCODE_TOO_MANY_COLUMNS),
153-
errmsg("cannot have more than %dkeys in statistics",
159+
errmsg("cannot have more than %dcolumns in statistics",
154160
STATS_MAX_DIMENSIONS)));
155161

156162
attnums[numcols]= ((Form_pg_attribute)GETSTRUCT(atttuple))->attnum;
157-
ReleaseSysCache(atttuple);
158163
numcols++;
164+
ReleaseSysCache(atttuple);
159165
}
160166

161167
/*
@@ -164,26 +170,27 @@ CreateStatistics(CreateStatsStmt *stmt)
164170
*/
165171
if (numcols<2)
166172
ereport(ERROR,
167-
(errcode(ERRCODE_TOO_MANY_COLUMNS),
168-
errmsg("statistics require at least 2 columns")));
173+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
174+
errmsg("extendedstatistics require at least 2 columns")));
169175

170176
/*
171-
* Sort the attnums, which makes detectingduplicities somewhat easier, and
177+
* Sort the attnums, which makes detectingduplicates somewhat easier, and
172178
* it does not hurt (it does not affect the efficiency, unlike for
173179
* indexes, for example).
174180
*/
175181
qsort(attnums,numcols,sizeof(int16),compare_int16);
176182

177183
/*
178-
*Look forduplicities in the list of columns. The attnums are sorted so
184+
*Check forduplicates in the list of columns. The attnums are sorted so
179185
* just check consecutive elements.
180186
*/
181187
for (i=1;i<numcols;i++)
182188
if (attnums[i]==attnums[i-1])
183189
ereport(ERROR,
184-
(errcode(ERRCODE_UNDEFINED_COLUMN),
190+
(errcode(ERRCODE_DUPLICATE_COLUMN),
185191
errmsg("duplicate column name in statistics definition")));
186192

193+
/* Form an int2vector representation of the sorted column list */
187194
stxkeys=buildint2vector(attnums,numcols);
188195

189196
/*
@@ -225,7 +232,7 @@ CreateStatistics(CreateStatsStmt *stmt)
225232
types[ntypes++]=CharGetDatum(STATS_EXT_NDISTINCT);
226233
if (build_dependencies)
227234
types[ntypes++]=CharGetDatum(STATS_EXT_DEPENDENCIES);
228-
Assert(ntypes>0);
235+
Assert(ntypes>0&&ntypes <=lengthof(types));
229236
stxkind=construct_array(types,ntypes,CHAROID,1, true,'c');
230237

231238
/*
@@ -240,7 +247,7 @@ CreateStatistics(CreateStatsStmt *stmt)
240247
values[Anum_pg_statistic_ext_stxkeys-1]=PointerGetDatum(stxkeys);
241248
values[Anum_pg_statistic_ext_stxkind-1]=PointerGetDatum(stxkind);
242249

243-
/* no statisticsbuild yet */
250+
/* no statisticsbuilt yet */
244251
nulls[Anum_pg_statistic_ext_stxndistinct-1]= true;
245252
nulls[Anum_pg_statistic_ext_stxdependencies-1]= true;
246253

@@ -260,7 +267,7 @@ CreateStatistics(CreateStatsStmt *stmt)
260267
relation_close(rel,NoLock);
261268

262269
/*
263-
* Add a dependency ona table, so that stats get dropped on DROP TABLE.
270+
* Add a dependency onthe table, so that stats get dropped on DROP TABLE.
264271
*/
265272
ObjectAddressSet(parentobject,RelationRelationId,relid);
266273
ObjectAddressSet(childobject,StatisticExtRelationId,statoid);
@@ -269,12 +276,15 @@ CreateStatistics(CreateStatsStmt *stmt)
269276
/*
270277
* Also add dependency on the schema. This is required to ensure that we
271278
* drop the statistics on DROP SCHEMA. This is not handled automatically
272-
* by DROP TABLE because the statistics are not an object in the table's
273-
* schema.
279+
* by DROP TABLE because the statistics might be in a different schema
280+
* from the table itself. (This definition is a bit bizarre for the
281+
* single-table case, but it will make more sense if/when we support
282+
* extended stats across multiple tables.)
274283
*/
275284
ObjectAddressSet(parentobject,NamespaceRelationId,namespaceId);
276285
recordDependencyOn(&childobject,&parentobject,DEPENDENCY_AUTO);
277286

287+
/* Return stats object's address */
278288
ObjectAddressSet(address,StatisticExtRelationId,statoid);
279289

280290
returnaddress;
@@ -287,13 +297,13 @@ void
287297
RemoveStatisticsById(OidstatsOid)
288298
{
289299
Relationrelation;
290-
Oidrelid;
291-
Relationrel;
292300
HeapTupletup;
293301
Form_pg_statistic_extstatext;
302+
Oidrelid;
294303

295304
/*
296-
* Delete the pg_statistic_ext tuple.
305+
* Delete the pg_statistic_ext tuple. Also send out a cache inval on the
306+
* associated table, so that dependent plans will be rebuilt.
297307
*/
298308
relation=heap_open(StatisticExtRelationId,RowExclusiveLock);
299309

@@ -305,14 +315,11 @@ RemoveStatisticsById(Oid statsOid)
305315
statext= (Form_pg_statistic_ext)GETSTRUCT(tup);
306316
relid=statext->stxrelid;
307317

308-
rel=heap_open(relid,AccessExclusiveLock);
318+
CacheInvalidateRelcacheByRelid(relid);
309319

310320
simple_heap_delete(relation,&tup->t_self);
311321

312-
CacheInvalidateRelcache(rel);
313-
314322
ReleaseSysCache(tup);
315323

316324
heap_close(relation,RowExclusiveLock);
317-
heap_close(rel,NoLock);
318325
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ CREATE STATISTICS s10 ON (unknown_column) FROM ndistinct;
163163
ERROR: column "unknown_column" referenced in statistics does not exist
164164
-- single column
165165
CREATE STATISTICS s10 ON (a) FROM ndistinct;
166-
ERROR: statistics require at least 2 columns
166+
ERROR:extendedstatistics require at least 2 columns
167167
-- single column, duplicated
168168
CREATE STATISTICS s10 ON (a,a) FROM ndistinct;
169169
ERROR: duplicate column name in statistics definition

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp