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

Commitcb02fcb

Browse files
committed
Fix bogus dependency handling for GENERATED expressions.
For GENERATED columns, we record all dependencies of the generationexpression as AUTO dependencies of the column itself. This meansthat the generated column is silently dropped if any dependencyis removed, even if CASCADE wasn't specified. This is at leasta POLA violation, but I think it's actually based on a misreadingof the standard. The standard does say that you can't drop adependent GENERATED column in RESTRICT mode; but that's buried downin a subparagraph, on a different page from some pseudocode thatmakes it look like an AUTO drop is being suggested.Change this to be more like the way that we handle regular defaultexpressions, ie record the dependencies as NORMAL dependencies ofthe pg_attrdef entry. Also, make the pg_attrdef entry's dependencyon the column itself be INTERNAL not AUTO. That has two effects:* the column will go away, not just lose its default, if anydependency of the expression is dropped with CASCADE. So wedon't need any special mechanism to make that happen.* it provides an additional cross-check preventing someone fromdropping the default expression without dropping the column.catversion bump because of change in the contents of pg_depend(which also requires a change in one information_schema view).Per bug #17439 from Kevin Humphreys. Although this is a longstandingbug, it seems impractical to back-patch because of the need forcatalog contents changes.Discussion:https://postgr.es/m/17439-7df4421197e928f0@postgresql.org
1 parent17f3bc0 commitcb02fcb

File tree

7 files changed

+113
-117
lines changed

7 files changed

+113
-117
lines changed

‎src/backend/catalog/information_schema.sql

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -514,16 +514,18 @@ CREATE VIEW column_column_usage AS
514514
CAST(ad.attnameAS sql_identifier)AS dependent_column
515515

516516
FROM pg_namespace n, pg_class c, pg_depend d,
517-
pg_attribute ac, pg_attribute ad
517+
pg_attribute ac, pg_attribute ad, pg_attrdef atd
518518

519519
WHEREn.oid=c.relnamespace
520520
ANDc.oid=ac.attrelid
521521
ANDc.oid=ad.attrelid
522-
ANDd.classid='pg_catalog.pg_class'::regclass
522+
ANDac.attnum<>ad.attnum
523+
ANDad.attrelid=atd.adrelid
524+
ANDad.attnum=atd.adnum
525+
ANDd.classid='pg_catalog.pg_attrdef'::regclass
523526
ANDd.refclassid='pg_catalog.pg_class'::regclass
524-
ANDd.objid=d.refobjid
525-
ANDc.oid=d.objid
526-
ANDd.objsubid=ad.attnum
527+
ANDd.objid=atd.oid
528+
ANDd.refobjid=ac.attrelid
527529
ANDd.refobjsubid=ac.attnum
528530
ANDad.attgenerated<>''
529531
AND pg_has_role(c.relowner,'USAGE');

‎src/backend/catalog/pg_attrdef.c

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -174,37 +174,23 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
174174

175175
/*
176176
* Make a dependency so that the pg_attrdef entry goes away if the column
177-
* (or whole table) is deleted.
177+
* (or whole table) is deleted. In the case of a generated column, make
178+
* it an internal dependency to prevent the default expression from being
179+
* deleted separately.
178180
*/
179181
colobject.classId=RelationRelationId;
180182
colobject.objectId=RelationGetRelid(rel);
181183
colobject.objectSubId=attnum;
182184

183-
recordDependencyOn(&defobject,&colobject,DEPENDENCY_AUTO);
185+
recordDependencyOn(&defobject,&colobject,
186+
attgenerated ?DEPENDENCY_INTERNAL :DEPENDENCY_AUTO);
184187

185188
/*
186189
* Record dependencies on objects used in the expression, too.
187190
*/
188-
if (attgenerated)
189-
{
190-
/*
191-
* Generated column: Dropping anything that the generation expression
192-
* refers to automatically drops the generated column.
193-
*/
194-
recordDependencyOnSingleRelExpr(&colobject,expr,RelationGetRelid(rel),
195-
DEPENDENCY_AUTO,
196-
DEPENDENCY_AUTO, false);
197-
}
198-
else
199-
{
200-
/*
201-
* Normal default: Dropping anything that the default refers to
202-
* requires CASCADE and drops the default only.
203-
*/
204-
recordDependencyOnSingleRelExpr(&defobject,expr,RelationGetRelid(rel),
205-
DEPENDENCY_NORMAL,
206-
DEPENDENCY_NORMAL, false);
207-
}
191+
recordDependencyOnSingleRelExpr(&defobject,expr,RelationGetRelid(rel),
192+
DEPENDENCY_NORMAL,
193+
DEPENDENCY_NORMAL, false);
208194

209195
/*
210196
* Post creation hook for attribute defaults.

‎src/backend/commands/tablecmds.c

Lines changed: 80 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -7899,6 +7899,7 @@ ATExecDropExpression(Relation rel, const char *colName, bool missing_ok, LOCKMOD
78997899
Form_pg_attribute attTup;
79007900
AttrNumberattnum;
79017901
Relationattrelation;
7902+
Oidattrdefoid;
79027903
ObjectAddress address;
79037904

79047905
attrelation = table_open(AttributeRelationId, RowExclusiveLock);
@@ -7936,71 +7937,44 @@ ATExecDropExpression(Relation rel, const char *colName, bool missing_ok, LOCKMOD
79367937
}
79377938
}
79387939

7940+
/*
7941+
* Mark the column as no longer generated. (The atthasdef flag needs to
7942+
* get cleared too, but RemoveAttrDefault will handle that.)
7943+
*/
79397944
attTup->attgenerated = '\0';
79407945
CatalogTupleUpdate(attrelation, &tuple->t_self, tuple);
79417946

79427947
InvokeObjectPostAlterHook(RelationRelationId,
79437948
RelationGetRelid(rel),
7944-
attTup->attnum);
7945-
ObjectAddressSubSet(address, RelationRelationId,
7946-
RelationGetRelid(rel), attnum);
7949+
attnum);
79477950
heap_freetuple(tuple);
79487951

79497952
table_close(attrelation, RowExclusiveLock);
79507953

7951-
CommandCounterIncrement();
7952-
7953-
RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, false, false);
7954-
79557954
/*
7956-
* Remove all dependencies of this (formerly generated) column on other
7957-
* columns in the same table. (See StoreAttrDefault() for which
7958-
* dependencies are created.) We don't expect there to be dependencies
7959-
* between columns of the same table for other reasons, so it's okay to
7960-
* remove all of them.
7955+
* Drop the dependency records of the GENERATED expression, in particular
7956+
* its INTERNAL dependency on the column, which would otherwise cause
7957+
* dependency.c to refuse to perform the deletion.
79617958
*/
7962-
{
7963-
RelationdepRel;
7964-
ScanKeyData key[3];
7965-
SysScanDesc scan;
7966-
HeapTupletup;
7967-
7968-
depRel = table_open(DependRelationId, RowExclusiveLock);
7969-
7970-
ScanKeyInit(&key[0],
7971-
Anum_pg_depend_classid,
7972-
BTEqualStrategyNumber, F_OIDEQ,
7973-
ObjectIdGetDatum(RelationRelationId));
7974-
ScanKeyInit(&key[1],
7975-
Anum_pg_depend_objid,
7976-
BTEqualStrategyNumber, F_OIDEQ,
7977-
ObjectIdGetDatum(RelationGetRelid(rel)));
7978-
ScanKeyInit(&key[2],
7979-
Anum_pg_depend_objsubid,
7980-
BTEqualStrategyNumber, F_INT4EQ,
7981-
Int32GetDatum(attnum));
7959+
attrdefoid = GetAttrDefaultOid(RelationGetRelid(rel), attnum);
7960+
if (!OidIsValid(attrdefoid))
7961+
elog(ERROR, "could not find attrdef tuple for relation %u attnum %d",
7962+
RelationGetRelid(rel), attnum);
7963+
(void) deleteDependencyRecordsFor(AttrDefaultRelationId, attrdefoid, false);
79827964

7983-
scan = systable_beginscan(depRel, DependDependerIndexId, true,
7984-
NULL, 3, key);
7985-
7986-
while (HeapTupleIsValid(tup = systable_getnext(scan)))
7987-
{
7988-
Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup);
7989-
7990-
if (depform->refclassid == RelationRelationId &&
7991-
depform->refobjid == RelationGetRelid(rel) &&
7992-
depform->refobjsubid != 0 &&
7993-
depform->deptype == DEPENDENCY_AUTO)
7994-
{
7995-
CatalogTupleDelete(depRel, &tup->t_self);
7996-
}
7997-
}
7998-
7999-
systable_endscan(scan);
7965+
/* Make above changes visible */
7966+
CommandCounterIncrement();
80007967

8001-
table_close(depRel, RowExclusiveLock);
8002-
}
7968+
/*
7969+
* Get rid of the GENERATED expression itself. We use RESTRICT here for
7970+
* safety, but at present we do not expect anything to depend on the
7971+
* default.
7972+
*/
7973+
RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT,
7974+
false, false);
80037975

7976+
ObjectAddressSubSet(address, RelationRelationId,
7977+
RelationGetRelid(rel), attnum);
80047978
return address;
80057979
}
80067980

@@ -12548,21 +12522,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1254812522
*/
1254912523
Assert(foundObject.objectSubId == 0);
1255012524
}
12551-
else if (relKind == RELKIND_RELATION &&
12552-
foundObject.objectSubId != 0 &&
12553-
get_attgenerated(foundObject.objectId, foundObject.objectSubId))
12554-
{
12555-
/*
12556-
* Changing the type of a column that is used by a
12557-
* generated column is not allowed by SQL standard. It
12558-
* might be doable with some thinking and effort.
12559-
*/
12560-
ereport(ERROR,
12561-
(errcode(ERRCODE_SYNTAX_ERROR),
12562-
errmsg("cannot alter type of a column used by a generated column"),
12563-
errdetail("Column \"%s\" is used by generated column \"%s\".",
12564-
colName, get_attname(foundObject.objectId, foundObject.objectSubId, false))));
12565-
}
1256612525
else
1256712526
{
1256812527
/* Not expecting any other direct dependencies... */
@@ -12625,13 +12584,39 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1262512584
break;
1262612585

1262712586
case OCLASS_DEFAULT:
12587+
{
12588+
ObjectAddress col = GetAttrDefaultColumnAddress(foundObject.objectId);
1262812589

12629-
/*
12630-
* Ignore the column's default expression, since we will fix
12631-
* it below.
12632-
*/
12633-
Assert(defaultexpr);
12634-
break;
12590+
if (col.objectId == RelationGetRelid(rel) &&
12591+
col.objectSubId == attnum)
12592+
{
12593+
/*
12594+
* Ignore the column's own default expression, which
12595+
* we will deal with below.
12596+
*/
12597+
Assert(defaultexpr);
12598+
}
12599+
else
12600+
{
12601+
/*
12602+
* This must be a reference from the expression of a
12603+
* generated column elsewhere in the same table.
12604+
* Changing the type of a column that is used by a
12605+
* generated column is not allowed by SQL standard, so
12606+
* just punt for now. It might be doable with some
12607+
* thinking and effort.
12608+
*/
12609+
ereport(ERROR,
12610+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
12611+
errmsg("cannot alter type of a column used by a generated column"),
12612+
errdetail("Column \"%s\" is used by generated column \"%s\".",
12613+
colName,
12614+
get_attname(col.objectId,
12615+
col.objectSubId,
12616+
false))));
12617+
}
12618+
break;
12619+
}
1263512620

1263612621
case OCLASS_STATISTIC_EXT:
1263712622

@@ -12694,9 +12679,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1269412679

1269512680
/*
1269612681
* Now scan for dependencies of this column on other things. The only
12697-
* thing we should find is the dependency on the column datatype, which we
12698-
* want to remove, possibly a collation dependency, and dependencies on
12699-
* other columns if it is a generated column.
12682+
* things we should find are the dependency on the column datatype and
12683+
* possibly a collation dependency. Those can be removed.
1270012684
*/
1270112685
ScanKeyInit(&key[0],
1270212686
Anum_pg_depend_classid,
@@ -12723,18 +12707,13 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1272312707
foundObject.objectId = foundDep->refobjid;
1272412708
foundObject.objectSubId = foundDep->refobjsubid;
1272512709

12726-
if (foundDep->deptype != DEPENDENCY_NORMAL &&
12727-
foundDep->deptype != DEPENDENCY_AUTO)
12710+
if (foundDep->deptype != DEPENDENCY_NORMAL)
1272812711
elog(ERROR, "found unexpected dependency type '%c'",
1272912712
foundDep->deptype);
1273012713
if (!(foundDep->refclassid == TypeRelationId &&
1273112714
foundDep->refobjid == attTup->atttypid) &&
1273212715
!(foundDep->refclassid == CollationRelationId &&
12733-
foundDep->refobjid == attTup->attcollation) &&
12734-
!(foundDep->refclassid == RelationRelationId &&
12735-
foundDep->refobjid == RelationGetRelid(rel) &&
12736-
foundDep->refobjsubid != 0)
12737-
)
12716+
foundDep->refobjid == attTup->attcollation))
1273812717
elog(ERROR, "found unexpected dependency for column: %s",
1273912718
getObjectDescription(&foundObject, false));
1274012719

@@ -12850,7 +12829,25 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1285012829
*/
1285112830
if (defaultexpr)
1285212831
{
12853-
/* Must make new row visible since it will be updated again */
12832+
/*
12833+
* If it's a GENERATED default, drop its dependency records, in
12834+
* particular its INTERNAL dependency on the column, which would
12835+
* otherwise cause dependency.c to refuse to perform the deletion.
12836+
*/
12837+
if (attTup->attgenerated)
12838+
{
12839+
Oidattrdefoid = GetAttrDefaultOid(RelationGetRelid(rel), attnum);
12840+
12841+
if (!OidIsValid(attrdefoid))
12842+
elog(ERROR, "could not find attrdef tuple for relation %u attnum %d",
12843+
RelationGetRelid(rel), attnum);
12844+
(void) deleteDependencyRecordsFor(AttrDefaultRelationId, attrdefoid, false);
12845+
}
12846+
12847+
/*
12848+
* Make updates-so-far visible, particularly the new pg_attribute row
12849+
* which will be updated again.
12850+
*/
1285412851
CommandCounterIncrement();
1285512852

1285612853
/*

‎src/bin/pg_dump/pg_dump_sort.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,10 +1203,10 @@ repairDependencyLoop(DumpableObject **loop,
12031203
* Loop of table with itself --- just ignore it.
12041204
*
12051205
* (Actually, what this arises from is a dependency of a table column on
1206-
* another column, whichhappens with generated columns; or a dependency
1207-
* of a table column on the whole table, which happens with partitioning.
1208-
* But we didn't pay attention to sub-object IDs while collecting the
1209-
* dependency data, so we can't see that here.)
1206+
* another column, whichhappened with generated columns before v15; or a
1207+
*dependencyof a table column on the whole table, which happens with
1208+
*partitioning.But we didn't pay attention to sub-object IDs while
1209+
*collecting thedependency data, so we can't see that here.)
12101210
*/
12111211
if (nLoop==1)
12121212
{

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/*yyyymmddN */
56-
#defineCATALOG_VERSION_NO202203171
56+
#defineCATALOG_VERSION_NO202203211
5757

5858
#endif

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,12 @@ SELECT * FROM gtest_tableoid;
477477

478478
-- drop column behavior
479479
CREATE TABLE gtest10 (a int PRIMARY KEY, b int, c int GENERATED ALWAYS AS (b * 2) STORED);
480-
ALTER TABLE gtest10 DROP COLUMN b;
480+
ALTER TABLE gtest10 DROP COLUMN b; -- fails
481+
ERROR: cannot drop column b of table gtest10 because other objects depend on it
482+
DETAIL: column c of table gtest10 depends on column b of table gtest10
483+
HINT: Use DROP ... CASCADE to drop the dependent objects too.
484+
ALTER TABLE gtest10 DROP COLUMN b CASCADE; -- drops c too
485+
NOTICE: drop cascades to column c of table gtest10
481486
\d gtest10
482487
Table "public.gtest10"
483488
Column | Type | Collation | Nullable | Default
@@ -519,6 +524,10 @@ SELECT a, c FROM gtest12s; -- allowed
519524
(2 rows)
520525

521526
RESET ROLE;
527+
DROP FUNCTION gf1(int); -- fail
528+
ERROR: cannot drop function gf1(integer) because other objects depend on it
529+
DETAIL: column c of table gtest12s depends on function gf1(integer)
530+
HINT: Use DROP ... CASCADE to drop the dependent objects too.
522531
DROP TABLE gtest11s, gtest12s;
523532
DROP FUNCTION gf1(int);
524533
DROP USER regress_user11;

‎src/test/regress/sql/generated.sql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,8 @@ SELECT * FROM gtest_tableoid;
231231

232232
-- drop column behavior
233233
CREATETABLEgtest10 (aintPRIMARY KEY, bint, cint GENERATED ALWAYSAS (b*2) STORED);
234-
ALTERTABLE gtest10 DROP COLUMN b;
234+
ALTERTABLE gtest10 DROP COLUMN b;-- fails
235+
ALTERTABLE gtest10 DROP COLUMN b CASCADE;-- drops c too
235236

236237
\d gtest10
237238

@@ -260,6 +261,7 @@ SELECT gf1(10); -- not allowed
260261
SELECT a, cFROM gtest12s;-- allowed
261262
RESET ROLE;
262263

264+
DROPFUNCTION gf1(int);-- fail
263265
DROPTABLE gtest11s, gtest12s;
264266
DROPFUNCTION gf1(int);
265267
DROPUSER regress_user11;

0 commit comments

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