diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index cab54c962c..94865b395b 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -378,3 +378,31 @@ RemoveStatisticsById(Oid statsOid) heap_close(relation, RowExclusiveLock); } + +/* + * Update a statistics object for ALTER COLUMN TYPE on a source column. + * + * This could throw an error if the type change can't be supported. + * If it can be supported, but the stats must be recomputed, a likely choice + * would be to set the relevant column(s) of the pg_statistic_ext tuple to + * null until the next ANALYZE. (Note that the type change hasn't actually + * happened yet, so one option that's *not* on the table is to recompute + * immediately.) + */ +void +UpdateStatisticsForTypeChange(Oid statsOid, Oid relationOid, int attnum, + Oid oldColumnType, Oid newColumnType) +{ + /* + * Currently, we don't actually need to do anything here. For both + * ndistinct and functional-dependencies stats, the on-disk representation + * is independent of the source column data types, and it is plausible to + * assume that the old statistic values will still be good for the new + * column contents. (Obviously, if the ALTER COLUMN TYPE has a USING + * expression that substantially alters the semantic meaning of the column + * values, this assumption could fail. But that seems like a corner case + * that doesn't justify zapping the stats in common cases.) + * + * Future types of extended stats will likely require us to work harder. + */ +} diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index cdcb94929a..b94db89b25 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9236,6 +9236,17 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, Assert(defaultexpr); break; + case OCLASS_STATISTIC_EXT: + + /* + * Give the extended-stats machinery a chance to fix anything + * that this column type change would break. + */ + UpdateStatisticsForTypeChange(foundObject.objectId, + RelationGetRelid(rel), attnum, + attTup->atttypid, targettype); + break; + case OCLASS_PROC: case OCLASS_TYPE: case OCLASS_CAST: @@ -9246,6 +9257,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, case OCLASS_OPERATOR: case OCLASS_OPCLASS: case OCLASS_OPFAMILY: + case OCLASS_AM: case OCLASS_AMOP: case OCLASS_AMPROC: case OCLASS_SCHEMA: @@ -9261,6 +9273,11 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, case OCLASS_USER_MAPPING: case OCLASS_DEFACL: case OCLASS_EXTENSION: + case OCLASS_EVENT_TRIGGER: + case OCLASS_PUBLICATION: + case OCLASS_PUBLICATION_REL: + case OCLASS_SUBSCRIPTION: + case OCLASS_TRANSFORM: /* * We don't expect any of these sorts of objects to depend on @@ -9270,9 +9287,10 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, getObjectDescription(&foundObject)); break; - default: - elog(ERROR, "unrecognized object class: %u", - foundObject.classId); + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new OCLASS hasn't been handled above. + */ } } diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index c323e81e6c..79f3be36e4 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -80,6 +80,9 @@ extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt); /* commands/statscmds.c */ extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt); extern void RemoveStatisticsById(Oid statsOid); +extern void UpdateStatisticsForTypeChange(Oid statsOid, + Oid relationOid, int attnum, + Oid oldColumnType, Oid newColumnType); /* commands/aggregatecmds.c */ extern ObjectAddress DefineAggregate(ParseState *pstate, List *name, List *args, bool oldstyle, diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index 564521957f..5fd1244bcb 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -479,4 +479,29 @@ EXPLAIN (COSTS OFF) Index Cond: ((a = 1) AND (b = '1'::text)) (5 rows) +-- check change of column type doesn't break it +ALTER TABLE functional_dependencies ALTER COLUMN c TYPE numeric; +EXPLAIN (COSTS OFF) + SELECT * FROM functional_dependencies WHERE a = 1 AND b = '1' AND c = 1; + QUERY PLAN +--------------------------------------------------- + Bitmap Heap Scan on functional_dependencies + Recheck Cond: ((a = 1) AND (b = '1'::text)) + Filter: (c = '1'::numeric) + -> Bitmap Index Scan on fdeps_ab_idx + Index Cond: ((a = 1) AND (b = '1'::text)) +(5 rows) + +ANALYZE functional_dependencies; +EXPLAIN (COSTS OFF) + SELECT * FROM functional_dependencies WHERE a = 1 AND b = '1' AND c = 1; + QUERY PLAN +--------------------------------------------------- + Bitmap Heap Scan on functional_dependencies + Recheck Cond: ((a = 1) AND (b = '1'::text)) + Filter: (c = '1'::numeric) + -> Bitmap Index Scan on fdeps_ab_idx + Index Cond: ((a = 1) AND (b = '1'::text)) +(5 rows) + RESET random_page_cost; diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql index a9dda051c2..4c2f514b93 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -266,6 +266,17 @@ ANALYZE functional_dependencies; EXPLAIN (COSTS OFF) SELECT * FROM functional_dependencies WHERE a = 1 AND b = '1'; +EXPLAIN (COSTS OFF) + SELECT * FROM functional_dependencies WHERE a = 1 AND b = '1' AND c = 1; + +-- check change of column type doesn't break it +ALTER TABLE functional_dependencies ALTER COLUMN c TYPE numeric; + +EXPLAIN (COSTS OFF) + SELECT * FROM functional_dependencies WHERE a = 1 AND b = '1' AND c = 1; + +ANALYZE functional_dependencies; + EXPLAIN (COSTS OFF) SELECT * FROM functional_dependencies WHERE a = 1 AND b = '1' AND c = 1;