Protect object creation from dropping referenced objects#1795
Merged
whitehawk merged 36 commits intoadb-6.x-devfrom Jul 10, 2025
Merged
Protect object creation from dropping referenced objects#1795whitehawk merged 36 commits intoadb-6.x-devfrom
whitehawk merged 36 commits intoadb-6.x-devfrom
Conversation
whitehawk
changed the title
whitehawk
changed the title
Member
Could you add a test for this? |
RekGRpth
reviewed
Jun 30, 2025
Member
|
Is it expected that the patch does not cover the case where the object being deleted is used in the function body? Type
diff --git a/src/test/isolation2/expected/dependency.out b/src/test/isolation2/expected/dependency.out
index f7a88622630..5d6df295a02 100644
--- a/src/test/isolation2/expected/dependency.out
+++ b/src/test/isolation2/expected/dependency.out
@@ -121,26 +121,28 @@ CREATE
1: begin;
BEGIN
-1: create function test_3_function(a test_3_type) returns text as $$ select 'Return ' || a; /**/ $$ language sql;
+1: create function test_3_function(a text) returns text as $$ select 'Return ' || a::test_3_type; /**/ $$ language sql;
CREATE
2&: drop type test_3_type; <waiting ...>
+FAILED: Forked command is not blocking; got output: DROP
1: commit;
COMMIT
2<: <... completed>
-ERROR: cannot drop type test_3_type because other objects depend on it
-DETAIL: function test_3_function(test_3_type) depends on type test_3_type
-HINT: Use DROP ... CASCADE to drop the dependent objects too.
+FAILED: Execution failed
1: select test_3_function('one');
- test_3_function
------------------
- Return one
-(1 row)
+ERROR: type "test_3_type" does not exist
+LINE 2: select 'Return ' || a::test_3_type; /**/
+ ^
+QUERY:
+ select 'Return ' || a::test_3_type; /**/
-drop type test_3_type cascade;
+CONTEXT: SQL function "test_3_function" during inlining
+
+drop function test_3_function(a text) cascade;
DROP
-- Check if dependency is dropped before the creation of the dependent object.
@@ -152,13 +154,21 @@ BEGIN
BEGIN
2: drop type test_3_type;
DROP
-1&: create function test_3_function(a test_3_type) returns text as $$ select 'Return ' || a; /**/ $$ language sql; <waiting ...>
+1&: create function test_3_function(a text) returns text as $$ select 'Return ' || a::test_3_type; /**/ $$ language sql; <waiting ...>
+FAILED: Forked command is not blocking; got output: CREATE
2: commit;
COMMIT
1<: <... completed>
-ERROR: cache lookup failed for type XXXXX (lsyscache.c:XXX)
-CONTEXT: SQL function "test_3_function"
+FAILED: Execution failed
+1: select test_3_function('one');
+ERROR: type "test_3_type" does not exist
+LINE 2: select 'Return ' || a::test_3_type; /**/
+ ^
+QUERY:
+ select 'Return ' || a::test_3_type; /**/
+
+CONTEXT: SQL function "test_3_function" during inlining
1: end;
END
diff --git a/src/test/isolation2/sql/dependency.sql b/src/test/isolation2/sql/dependency.sql
index b8bfe65a214..fe3afe39140 100644
--- a/src/test/isolation2/sql/dependency.sql
+++ b/src/test/isolation2/sql/dependency.sql
@@ -84,8 +84,8 @@ $$ language sql;
create type test_3_type as enum ('one', 'two');
1: begin;
-1: create function test_3_function(a test_3_type) returns text as $$
- select 'Return ' || a; /**/
+1: create function test_3_function(a text) returns text as $$
+ select 'Return ' || a::test_3_type; /**/
$$ language sql;
2&: drop type test_3_type;
@@ -96,19 +96,20 @@ $$ language sql;
1: select test_3_function('one');
-drop type test_3_type cascade;
+drop function test_3_function(a text) cascade;
-- Check if dependency is dropped before the creation of the dependent object.
create type test_3_type as enum ('one', 'two');
1: begin;
2: begin;
2: drop type test_3_type;
-1&: create function test_3_function(a test_3_type) returns text as $$
- select 'Return ' || a; /**/
+1&: create function test_3_function(a text) returns text as $$
+ select 'Return ' || a::test_3_type; /**/
$$ language sql;
2: commit;
1<:
+1: select test_3_function('one');
1: end;
-- Case 4. Function dependency on the language.Table
diff --git a/src/test/isolation2/expected/dependency.out b/src/test/isolation2/expected/dependency.out
index f7a88622630..ad632597a43 100644
--- a/src/test/isolation2/expected/dependency.out
+++ b/src/test/isolation2/expected/dependency.out
@@ -65,53 +65,53 @@ ERROR: schema "test_1_schema" does not exist (seg0 127.0.1.1:6002 pid=2715327)
END
-- Case 2. Function dependency on the return type.
-create type test_2_type as (a int);
+create table test_2_type (a int);
CREATE
1: begin;
BEGIN
-1: create function test_2_function() returns setof test_2_type as $$ select i from generate_series(1,5)i; /**/ $$ language sql;
+1: create function test_2_function() returns setof int as $$ select * from test_2_type; /**/ $$ language sql;
CREATE
-2&: drop type test_2_type; <waiting ...>
+2&: drop table test_2_type; <waiting ...>
1: commit;
COMMIT
2<: <... completed>
-ERROR: cannot drop type test_2_type because other objects depend on it
-DETAIL: function test_2_function() depends on type test_2_type
-HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP
1: select test_2_function();
- test_2_function
------------------
- (1)
- (2)
- (3)
- (4)
- (5)
-(5 rows)
-
-drop type test_2_type cascade;
+ERROR: relation "test_2_type" does not exist
+LINE 2: select * from test_2_type; /**/
+ ^
+QUERY:
+ select * from test_2_type; /**/
+
+CONTEXT: SQL function "test_2_function" during startup
+
+drop table test_2_type cascade;
+ERROR: table "test_2_type" does not exist
+drop function test_2_function();
DROP
-- Check if dependency is dropped before the creation of the dependent object.
-create type test_2_type as (a int);
+create table test_2_type (a int);
CREATE
1: begin;
BEGIN
2: begin;
BEGIN
-2: drop type test_2_type;
+2: drop table test_2_type;
DROP
-1&: create function test_2_function() returns setof test_2_type as $$ select i from generate_series(1,5)i; /**/ $$ language sql; <waiting ...>
+1&: create function test_2_function() returns setof int as $$ select * from test_2_type; /**/ $$ language sql; <waiting ...>
2: commit;
COMMIT
1<: <... completed>
-ERROR: cache lookup failed for type XXXXX (format_type.c:XXX)
-CONTEXT: SQL function "test_2_function"
+ERROR: relation "test_2_type" does not exist
+LINE 2: select * from test_2_type; /**/
+ ^
1: end;
END
diff --git a/src/test/isolation2/sql/dependency.sql b/src/test/isolation2/sql/dependency.sql
index b8bfe65a214..a1fdb32b9fb 100644
--- a/src/test/isolation2/sql/dependency.sql
+++ b/src/test/isolation2/sql/dependency.sql
@@ -50,14 +50,14 @@ $$ language sql;
1: end;
-- Case 2. Function dependency on the return type.
-create type test_2_type as (a int);
+create table test_2_type (a int);
1: begin;
-1: create function test_2_function() returns setof test_2_type as $$
- select i from generate_series(1,5)i; /**/
+1: create function test_2_function() returns setof int as $$
+ select * from test_2_type; /**/
$$ language sql;
-2&: drop type test_2_type;
+2&: drop table test_2_type;
1: commit;
@@ -65,15 +65,16 @@ $$ language sql;
1: select test_2_function();
-drop type test_2_type cascade;
+drop table test_2_type cascade;
+drop function test_2_function();
-- Check if dependency is dropped before the creation of the dependent object.
-create type test_2_type as (a int);
+create table test_2_type (a int);
1: begin;
2: begin;
-2: drop type test_2_type;
-1&: create function test_2_function() returns setof test_2_type as $$
- select i from generate_series(1,5)i; /**/
+2: drop table test_2_type;
+1&: create function test_2_function() returns setof int as $$
+ select * from test_2_type; /**/
$$ language sql;
2: commit;Default
diff --git a/src/test/isolation2/expected/dependency.out b/src/test/isolation2/expected/dependency.out
index f7a88622630..75024f2494e 100644
--- a/src/test/isolation2/expected/dependency.out
+++ b/src/test/isolation2/expected/dependency.out
@@ -215,26 +215,27 @@ CREATE
1: begin;
BEGIN
-1: create function test_5_function(a text default test5_default_value_function()) returns text as $$ begin return a; /**/ end $$ language plpgsql;
+1: create function test_5_function(a text default 'qwe') returns text as $$ declare b text default test5_default_value_function(); /**/ begin return a || b; /**/ end $$ language plpgsql;
CREATE
2&: drop function test5_default_value_function(); <waiting ...>
+FAILED: Forked command is not blocking; got output: DROP
1: commit;
COMMIT
2<: <... completed>
-ERROR: cannot drop function test5_default_value_function() because other objects depend on it
-DETAIL: function test_5_function(text) depends on function test5_default_value_function()
-HINT: Use DROP ... CASCADE to drop the dependent objects too.
+FAILED: Execution failed
1: select test_5_function();
- test_5_function
------------------
- test
-(1 row)
+ERROR: function test5_default_value_function() does not exist
+LINE 1: SELECT test5_default_value_function()
+ ^
+HINT: No function matches the given name and argument types. You might need to add explicit type casts.
+QUERY: SELECT test5_default_value_function()
+CONTEXT: PL/pgSQL function test_5_function(text) line 4 during statement block local variable initialization
-drop function test5_default_value_function() cascade;
+drop function test_5_function(a text) cascade;
DROP
-- Check if dependency is dropped before the creation of the dependent object.
@@ -247,13 +248,13 @@ BEGIN
BEGIN
2: drop function test5_default_value_function();
DROP
-1&: create function test_5_function(a text default test5_default_value_function()) returns text as $$ begin return a; /**/ end $$ language plpgsql; <waiting ...>
+1&: create function test_5_function(a text default 'qwe') returns text as $$ declare b text default test5_default_value_function(); /**/ begin return a || b; /**/ end $$ language plpgsql; <waiting ...>
+FAILED: Forked command is not blocking; got output: CREATE
2: commit;
COMMIT
1<: <... completed>
-ERROR: function test5_default_value_function() does not exist (seg0 127.0.1.1:6002 pid=2718914)
-HINT: No function matches the given name and argument types. You might need to add explicit type casts.
+FAILED: Execution failed
1: end;
END
diff --git a/src/test/isolation2/sql/dependency.sql b/src/test/isolation2/sql/dependency.sql
index b8bfe65a214..beee8115eff 100644
--- a/src/test/isolation2/sql/dependency.sql
+++ b/src/test/isolation2/sql/dependency.sql
@@ -152,10 +152,12 @@ create function test5_default_value_function() returns text as $$
$$ language sql;
1: begin;
-1: create function test_5_function(a text default test5_default_value_function()) returns text as
+1: create function test_5_function(a text default 'qwe') returns text as
$$
+declare
+ b text default test5_default_value_function(); /**/
begin
- return a; /**/
+ return a || b; /**/
end
$$ language plpgsql;
@@ -167,7 +169,7 @@ $$ language plpgsql;
1: select test_5_function();
-drop function test5_default_value_function() cascade;
+drop function test_5_function(a text) cascade;
-- Check if dependency is dropped before the creation of the dependent object.
create function test5_default_value_function() returns text as $$
@@ -177,10 +179,12 @@ $$ language sql;
1: begin;
2: begin;
2: drop function test5_default_value_function();
-1&: create function test_5_function(a text default test5_default_value_function()) returns text as
+1&: create function test_5_function(a text default 'qwe') returns text as
$$
+declare
+ b text default test5_default_value_function(); /**/
begin
- return a; /**/
+ return a || b; /**/
end
$$ language plpgsql;
|
Member
|
Could you add tests for creating a view that depends on the function being removed, and for changing the column type of a table, and also for the rules that depend on the table being dropped? |
Member
|
Could you benchmark the performance degradation due to adding new extra locks? |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Author
I usually do not add it, as it can be obtained from the branch name. And it is not mandated by the review checklist.
I've added check of pg_locks into the first couple of test cases.
I've done performance testing with the help of functions below: commands used for perf measurement: results without the patch: total 204,82 sec. results with the patch: total 205,26 sec. So the difference between total numbers looks neglectable.
Added
Yes, according to postgres documentation:
Added. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Author
Ok, test code in sqltest commandfor i in {1..10};
do
seq 10 | parallel --bar -j 10 --gnu "DUMMY={}; TIME=\"\t%E sec\" time psql postgres -c \"begin; select create_many_tables(); rollback;\" > /dev/null"
doneTest results without the patchIn summ - 04h:03m:04s,37 (14584,37 sec) Test results with the patchIn summ - 04h:17m:50s,96 (15470,96 sec), the delta is ~6% slower. And one more round with the patchIn summ - 03h:45m:55s,80 (13555.8 sec) which is ~7% faster than without the patch. So, in general looks ok from my point of view.
Without the patch: With the patch: The locks delta here (1000 locks) is expected, as we are creating 1000 tables, and each one depends on its own type. And, as you can see, even without the patch, we can easily hit the value of thousands of locks, if we need to create thousands of tables. Therefore, restore of a large database with many objects in a single transaction anyway can hit the limit.
Added test for |
hilltracer
previously approved these changes
Jul 9, 2025
hilltracer
left a comment
hilltracer
left a comment
There was a problem hiding this comment.
Thanks for the detailed experiments. I think we're now ready to present our results for Tom Lane and co.
RekGRpth
reviewed
Jul 9, 2025
RekGRpth
reviewed
Jul 10, 2025
RekGRpth
reviewed
Jul 10, 2025
RekGRpth
reviewed
Jul 10, 2025
RekGRpth
reviewed
Jul 10, 2025
RekGRpth
reviewed
Jul 10, 2025
RekGRpth
reviewed
Jul 10, 2025
RekGRpth
reviewed
Jul 10, 2025
RekGRpth
approved these changes
Jul 10, 2025
hilltracer
approved these changes
Jul 10, 2025
whitehawk
added a commit
that referenced
this pull request
Jul 11, 2025
Problem description:
If an object had been created inside a transaction and it had a dependency on
some other object (like schema, custom data type, etc), and that referenced
object had been dropped from another transaction before the first transaction
had been committed, the new object was broken.
Ex. steps:
```
Session 1: create type test_type as enum ('one', 'two');
Session 1: begin;
Session 1: create table test_table(a test_type);
Session 2: drop type test_type;
Session 1: commit;
Session 1: select * from test_table;
```
gave result:
```
ERROR: cache lookup failed for type 16385 (lsyscache.c:2796)
```
The issue reproduced for all dependencies that are registered in 'pg_depend'.
Root cause:
No protection from stealing the objects that are marked as dependencies in
'pg_depend' before the transaction is committed.
Fix:
Lock the referenced object with AccessShareLock when the dependency is created.
Lock is done only for 'normal' dependencies (which means here 'normal
relationship between separately-created objects', when the dependent object may
be dropped without affecting the referenced object, while the referenced object
may only be dropped with CASCADE, in which case the dependent object is dropped
too) and 'auto' (meaning that the dependent object can be dropped separately
from the referenced object, and should be automatically dropped if the
referenced object is dropped).
After the lock is released, do a recheck of the object's presence and error out
if the object was dropped while we waited for the lock.
(cherry picked from commit 769ddb3)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Protect object creation from dropping referenced objects
Problem description:
If an object had been created inside a transaction and it had a dependency on
some other object (like schema, custom data type, etc), and that referenced
object had been dropped from another transaction before the first transaction
had been committed, the new object was broken.
Ex. steps:
gave result:
The issue reproduced for all dependencies that are registered in 'pg_depend'.
Root cause:
No protection from stealing the objects that are marked as dependencies in
'pg_depend' before the transaction is committed.
Fix:
Lock the referenced object with AccessShareLock when the dependency is created.
Lock is done only for 'normal' dependencies (which means here 'normal
relationship between separately-created objects', when the dependent object may
be dropped without affecting the referenced object, while the referenced object
may only be dropped with CASCADE, in which case the dependent object is dropped
too) and 'auto' (meaning that the dependent object can be dropped separately
from the referenced object, and should be automatically dropped if the
referenced object is dropped).
After the lock is released, do a recheck of the object's presence and error out
if the object was dropped while we waited for the lock.