8000 Rethink the dependencies recorded for FieldSelect/FieldStore nodes. · MSPawanRanjith/postgres@adcfa7b · GitHub
[go: up one dir, main page]
Skip to content

Commit adcfa7b

Browse files
tglsfdctglsfdc
committed
Rethink the dependencies recorded for FieldSelect/FieldStore nodes.
On closer investigation, commits f3ea3e3 et al were a few bricks shy of a load. What we need is not so much to lock down the result type of a FieldSelect, as to lock down the existence of the column it's trying to extract. Otherwise, we can break it by dropping that column. The dependency on the result type is then held indirectly through the column, and doesn't need to be recorded explicitly. Out of paranoia, I left in the code to record a dependency on the result type, but it's used only if we can't identify the pg_class OID for the column. That shouldn't ever happen right now, AFAICS, but it seems possible that in future the input node could be marked as being of type RECORD rather than some specific composite type. Likewise for FieldStore. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/22571.1509064146@sss.pgh.pa.us
1 parent f39fc2b commit adcfa7b

File tree

3 files changed

+55
-6
lines changed

3 files changed

+55
-6
lines changed

‎src/backend/catalog/dependency.c

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,10 +1642,24 @@ find_expr_references_walker(Node *node,
16421642
else if (IsA(node, FieldSelect))
16431643
{
16441644
FieldSelect *fselect = (FieldSelect *) node;
1645+
Oid argtype = exprType((Node *) fselect->arg);
1646+
Oid reltype = get_typ_typrelid(argtype);
16451647

1646-
/* result type might not appear anywhere else in expression */
1647-
add_object_address(OCLASS_TYPE, fselect->resulttype, 0,
1648-
context->addrs);
1648+
/*
1649+
* We need a dependency on the specific column named in FieldSelect,
1650+
* assuming we can identify the pg_class OID for it. (Probably we
1651+
* always can at the moment, but in future it might be possible for
1652+
* argtype to be RECORDOID.) If we can make a column dependency then
1653+
* we shouldn't need a dependency on the column's type; but if we
1654+
* can't, make a dependency on the type, as it might not appear
1655+
* anywhere else in the expression.
1656+
*/
1657+
if (OidIsValid(reltype))
1658+
add_object_address(OCLASS_CLASS, reltype, fselect->fieldnum,
1659+
context->addrs);
1660+
else
1661+
add_object_address(OCLASS_TYPE, fselect->resulttype, 0,
1662+
context->addrs);
16491663
/* the collation might not be referenced anywhere else, either */
16501664
if (OidIsValid(fselect->resultcollid) &&
16511665
fselect->resultcollid != DEFAULT_COLLATION_OID)
@@ -1655,10 +1669,20 @@ find_expr_references_walker(Node *node,
16551669
else if (IsA(node, FieldStore))
16561670
{
16571671
FieldStore *fstore = (FieldStore *) node;
1672+
Oid reltype = get_typ_typrelid(fstore->resulttype);
16581673

1659-
/* result type might not appear anywhere else in expression */
1660-
add_object_address(OCLASS_TYPE, fstore->resulttype, 0,
1661-
context->addrs);
1674+
/* similar considerations to FieldSelect, but multiple column(s) */
1675+
if (OidIsValid(reltype))
1676+
{
1677+
ListCell *l;
1678+
1679+
foreach(l, fstore->fieldnums)
1680+
add_object_address(OCLASS_CLASS, reltype, lfirst_int(l),
1681+
context->addrs);
1682+
}
1683+
else
1684+
add_object_address(OCLASS_TYPE, fstore->resulttype, 0,
1685+
context->addrs);
16621686
}
16631687
else if (IsA(node, RelabelType))
16641688
{

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2488,6 +2488,23 @@ Table "public.test_tbl2_subclass"
24882488
Inherits: test_tbl2
24892489

24902490
DROP TABLE test_tbl2_subclass;
2491+
CREATE TYPE test_typex AS (a int, b text);
2492+
CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0));
2493+
ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails
2494+
ERROR: cannot drop composite type test_typex column a because other objects depend on it
2495+
DETAIL: constraint test_tblx_y_check on table test_tblx depends on composite type test_typex column a
2496+
HINT: Use DROP ... CASCADE to drop the dependent objects too.
2497+
ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE;
2498+
NOTICE: drop cascades to constraint test_tblx_y_check on table test_tblx
2499+
\d test_tblx
2500+
Table "public.test_tblx"
2501+
Column | Type | Modifiers
2502+
--------+------------+-----------
2503+
x | integer |
2504+
y | test_typex |
2505+
2506+
DROP TABLE test_tblx;
2507+
DROP TYPE test_typex;
24912508
-- This test isn't that interesting on its own, but the purpose is to leave
24922509
-- behind a table to test pg_upgrade with. The table has a composite type
24932510
-- column in it, and the composite type has a dropped attribute.

‎src/test/regress/sql/alter_table.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,6 +1558,14 @@ ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
15581558

15591559
DROP TABLE test_tbl2_subclass;
15601560

1561+
CREATE TYPE test_typex AS (a int, b text);
1562+
CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0));
1563+
ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails
1564+
ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE;
1565+
\d test_tblx
1566+
DROP TABLE test_tblx;
1567+
DROP TYPE test_typex;
1568+
15611569
-- This test isn't that interesting on its own, but the purpose is to leave
15621570
-- behind a table to test pg_upgrade with. The table has a composite type
15631571
-- column in it, and the composite type has a dropped attribute.

0 commit comments

Comments
 (0)
0