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

Commitb5b0db1

Browse files
committed
Fix handling of extended statistics during ALTER COLUMN TYPE.
ALTER COLUMN TYPE on a column used by a statistics object fails sincecommit928c4de, because the relevant switch in ATExecAlterColumnTypeis unprepared for columns to have dependencies from OCLASS_STATISTIC_EXTobjects.Although the existing types of extended statistics don't actually need usto do any work for a column type change, it seems completely indefensiblethat that assumption is hidden behind the failure of an unrelated moduleto contain any code for the case. Hence, create and call an API functionin statscmds.c where the assumption can be explained, and where we couldadd code to deal with the problem when it inevitably becomes real.Also, the reason this wasn't handled before, neither for extended statsnor for the last half-dozen new OCLASS kinds :-(, is that the default:in that switch suppresses compiler warnings, allowing people to miss theneed to consider it when adding an OCLASS. We don't really need a defaultbecause surely getObjectClass should only return valid values of the enum;so remove it, and add the missed OCLASS entries where they should be.Discussion:https://postgr.es/m/20170512221010.nglatgt5azzdxjlj@alvherre.pgsql
1 parent65b655b commitb5b0db1

File tree

5 files changed

+88
-3
lines changed

5 files changed

+88
-3
lines changed

‎src/backend/commands/statscmds.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,3 +378,31 @@ RemoveStatisticsById(Oid statsOid)
378378

379379
heap_close(relation,RowExclusiveLock);
380380
}
381+
382+
/*
383+
* Update a statistics object for ALTER COLUMN TYPE on a source column.
384+
*
385+
* This could throw an error if the type change can't be supported.
386+
* If it can be supported, but the stats must be recomputed, a likely choice
387+
* would be to set the relevant column(s) of the pg_statistic_ext tuple to
388+
* null until the next ANALYZE. (Note that the type change hasn't actually
389+
* happened yet, so one option that's *not* on the table is to recompute
390+
* immediately.)
391+
*/
392+
void
393+
UpdateStatisticsForTypeChange(OidstatsOid,OidrelationOid,intattnum,
394+
OidoldColumnType,OidnewColumnType)
395+
{
396+
/*
397+
* Currently, we don't actually need to do anything here. For both
398+
* ndistinct and functional-dependencies stats, the on-disk representation
399+
* is independent of the source column data types, and it is plausible to
400+
* assume that the old statistic values will still be good for the new
401+
* column contents. (Obviously, if the ALTER COLUMN TYPE has a USING
402+
* expression that substantially alters the semantic meaning of the column
403+
* values, this assumption could fail. But that seems like a corner case
404+
* that doesn't justify zapping the stats in common cases.)
405+
*
406+
* Future types of extended stats will likely require us to work harder.
407+
*/
408+
}

‎src/backend/commands/tablecmds.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9236,6 +9236,17 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
92369236
Assert(defaultexpr);
92379237
break;
92389238

9239+
caseOCLASS_STATISTIC_EXT:
9240+
9241+
/*
9242+
* Give the extended-stats machinery a chance to fix anything
9243+
* that this column type change would break.
9244+
*/
9245+
UpdateStatisticsForTypeChange(foundObject.objectId,
9246+
RelationGetRelid(rel),attnum,
9247+
attTup->atttypid,targettype);
9248+
break;
9249+
92399250
caseOCLASS_PROC:
92409251
caseOCLASS_TYPE:
92419252
caseOCLASS_CAST:
@@ -9246,6 +9257,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
92469257
caseOCLASS_OPERATOR:
92479258
caseOCLASS_OPCLASS:
92489259
caseOCLASS_OPFAMILY:
9260+
caseOCLASS_AM:
92499261
caseOCLASS_AMOP:
92509262
caseOCLASS_AMPROC:
92519263
caseOCLASS_SCHEMA:
@@ -9261,6 +9273,11 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
92619273
caseOCLASS_USER_MAPPING:
92629274
caseOCLASS_DEFACL:
92639275
caseOCLASS_EXTENSION:
9276+
caseOCLASS_EVENT_TRIGGER:
9277+
caseOCLASS_PUBLICATION:
9278+
caseOCLASS_PUBLICATION_REL:
9279+
caseOCLASS_SUBSCRIPTION:
9280+
caseOCLASS_TRANSFORM:
92649281

92659282
/*
92669283
* We don't expect any of these sorts of objects to depend on
@@ -9270,9 +9287,10 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
92709287
getObjectDescription(&foundObject));
92719288
break;
92729289

9273-
default:
9274-
elog(ERROR,"unrecognized object class: %u",
9275-
foundObject.classId);
9290+
/*
9291+
* There's intentionally no default: case here; we want the
9292+
* compiler to warn if a new OCLASS hasn't been handled above.
9293+
*/
92769294
}
92779295
}
92789296

‎src/include/commands/defrem.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt);
8080
/* commands/statscmds.c */
8181
externObjectAddressCreateStatistics(CreateStatsStmt*stmt);
8282
externvoidRemoveStatisticsById(OidstatsOid);
83+
externvoidUpdateStatisticsForTypeChange(OidstatsOid,
84+
OidrelationOid,intattnum,
85+
OidoldColumnType,OidnewColumnType);
8386

8487
/* commands/aggregatecmds.c */
8588
externObjectAddressDefineAggregate(ParseState*pstate,List*name,List*args,boololdstyle,

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,4 +479,29 @@ EXPLAIN (COSTS OFF)
479479
Index Cond: ((a = 1) AND (b = '1'::text))
480480
(5 rows)
481481

482+
-- check change of column type doesn't break it
483+
ALTER TABLE functional_dependencies ALTER COLUMN c TYPE numeric;
484+
EXPLAIN (COSTS OFF)
485+
SELECT * FROM functional_dependencies WHERE a = 1 AND b = '1' AND c = 1;
486+
QUERY PLAN
487+
---------------------------------------------------
488+
Bitmap Heap Scan on functional_dependencies
489+
Recheck Cond: ((a = 1) AND (b = '1'::text))
490+
Filter: (c = '1'::numeric)
491+
-> Bitmap Index Scan on fdeps_ab_idx
492+
Index Cond: ((a = 1) AND (b = '1'::text))
493+
(5 rows)
494+
495+
ANALYZE functional_dependencies;
496+
EXPLAIN (COSTS OFF)
497+
SELECT * FROM functional_dependencies WHERE a = 1 AND b = '1' AND c = 1;
498+
QUERY PLAN
499+
---------------------------------------------------
500+
Bitmap Heap Scan on functional_dependencies
501+
Recheck Cond: ((a = 1) AND (b = '1'::text))
502+
Filter: (c = '1'::numeric)
503+
-> Bitmap Index Scan on fdeps_ab_idx
504+
Index Cond: ((a = 1) AND (b = '1'::text))
505+
(5 rows)
506+
482507
RESET random_page_cost;

‎src/test/regress/sql/stats_ext.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,17 @@ ANALYZE functional_dependencies;
266266
EXPLAIN (COSTS OFF)
267267
SELECT*FROM functional_dependenciesWHERE a=1AND b='1';
268268

269+
EXPLAIN (COSTS OFF)
270+
SELECT*FROM functional_dependenciesWHERE a=1AND b='1'AND c=1;
271+
272+
-- check change of column type doesn't break it
273+
ALTERTABLE functional_dependencies ALTER COLUMN c TYPEnumeric;
274+
275+
EXPLAIN (COSTS OFF)
276+
SELECT*FROM functional_dependenciesWHERE a=1AND b='1'AND c=1;
277+
278+
ANALYZE functional_dependencies;
279+
269280
EXPLAIN (COSTS OFF)
270281
SELECT*FROM functional_dependenciesWHERE a=1AND b='1'AND c=1;
271282

0 commit comments

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