Skip to content

Commit

Permalink
Fix alter table drop column drops trigger on the table (#3225) (#3276)
Browse files Browse the repository at this point in the history
In postgres trigger and functions are two different objects. You first define a function and then use it in your trigger definition. In babelfish we handle this internally so that for users it feels like a single object. As part of this internal handing we also drop the function created for this trigger as part of post object access hook. Currently the condition for DROP TABLE was incomplete and even for DROP COLUMN we would drop the triggers associated with the table.
As a fix add another condition to only execute this internal drop of triggers for DROP TABLE.

Issues Resolved: BABEL-5417
Signed-off-by: Tanzeel Khan <[email protected]>
  • Loading branch information
tanscorpio7 authored Dec 18, 2024
1 parent d08cf7c commit 1192691
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 22 deletions.
2 changes: 1 addition & 1 deletion contrib/babelfishpg_tsql/src/tablecmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ lookup_and_drop_triggers(ObjectAccessType access, Oid classId,
return;

/* We only want to execute this function for the DROP TABLE case */
if (classId != RelationRelationId || access != OAT_DROP)
if (classId != RelationRelationId || access != OAT_DROP || subId != 0)
return;

/*
Expand Down
33 changes: 33 additions & 0 deletions test/JDBC/expected/alter_table.out
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,36 @@ GO

drop table trans2
GO

--------------------------- BABEL-5417 ---------------------------
------- DROP COLUMN SHOULD NOT DROP TRIGGERS ON THE TABLE --------
CREATE TABLE babel_5417(a int, b int);
GO
CREATE TRIGGER babel_5417_trg
ON babel_5417
AFTER INSERT AS SELECT 1
GO
SELECT tgname FROM pg_trigger WHERE tgname LIKE 'babel_5417%';
GO
~~START~~
varchar
babel_5417_trg
~~END~~

ALTER TABLE babel_5417 DROP COLUMN a
GO
SELECT tgname FROM pg_trigger WHERE tgname LIKE 'babel_5417%';
GO
~~START~~
varchar
babel_5417_trg
~~END~~

DROP TABLE babel_5417
GO
SELECT tgname FROM pg_trigger WHERE tgname LIKE 'babel_5417%';
GO
~~START~~
varchar
~~END~~

10 changes: 0 additions & 10 deletions test/JDBC/expected/db_owner-vu-verify.out
Original file line number Diff line number Diff line change
Expand Up @@ -305,16 +305,6 @@ go
alter procedure dbo.dbowner__p0 as select 20
go

-- BABEL-5417: Trigger gets dropped if we drop a column in table
create trigger dbo.dbowner__trg0
on dbo.dbowner__t0
after insert
as
begin
select 'New row inserted'
end
go

-- Member of db_owner role should be allowed to rename objects
exec sp_rename 'dbo.dbowner__t0.x', 'x_renamed', 'column'
go
Expand Down
23 changes: 22 additions & 1 deletion test/JDBC/input/alter_table.sql
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,25 @@ ALTER TABLE trans2 DROP COLUMN AddDate
GO

drop table trans2
GO
GO

--------------------------- BABEL-5417 ---------------------------
------- DROP COLUMN SHOULD NOT DROP TRIGGERS ON THE TABLE --------
CREATE TABLE babel_5417(a int, b int);
GO
CREATE TRIGGER babel_5417_trg
ON babel_5417
AFTER INSERT AS SELECT 1
GO
SELECT tgname FROM pg_trigger WHERE tgname LIKE 'babel_5417%';
GO
ALTER TABLE babel_5417 DROP COLUMN a
GO
SELECT tgname FROM pg_trigger WHERE tgname LIKE 'babel_5417%';
GO
DROP TABLE babel_5417
GO
SELECT tgname FROM pg_trigger WHERE tgname LIKE 'babel_5417%';
GO
------------------------------------------------------------------
------------------------------------------------------------------
10 changes: 0 additions & 10 deletions test/JDBC/input/db_owner-vu-verify.mix
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,6 @@ go
alter procedure dbo.dbowner__p0 as select 20
go

-- BABEL-5417: Trigger gets dropped if we drop a column in table
create trigger dbo.dbowner__trg0
on dbo.dbowner__t0
after insert
as
begin
select 'New row inserted'
end
go

-- Member of db_owner role should be allowed to rename objects
exec sp_rename 'dbo.dbowner__t0.x', 'x_renamed', 'column'
go
Expand Down

0 comments on commit 1192691

Please sign in to comment.