Re: BUG #19686: Rolling back SET TABLESPACE

[shihao zhong]

Hi Manu,

v5 looks good. One small thing.

A sql_drop event trigger also runs after the ALTER, and v5 only
checks ddl_command_end. Andres said a superuser can keep the pieces
here, so maybe neither needs a check. But if you check one, the
other should be checked too.

    CREATE TABLE t (a int, b int);
    CREATE INDEX ON t (a);
    CREATE FUNCTION f() RETURNS event_trigger LANGUAGE plpgsql AS $$
    BEGIN
      INSERT INTO t VALUES (5);
      RAISE EXCEPTION 'fail';
    END $$;
    CREATE EVENT TRIGGER e ON sql_drop EXECUTE FUNCTION f();
    ALTER TABLE t SET TABLESPACE ts, DROP COLUMN b;
    DROP EVENT TRIGGER e;
    INSERT INTO t VALUES (7);
    SET enable_seqscan = off;
    SELECT count(*) FROM t WHERE a = 5;

This returns 1 with v5, and 0 with the attached diff on top of v5.

Also, ALTER TABLE ALL IN TABLESPACE always copies the indexes. Is that on
purpose?

Thanks,
Shihao

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index bd213461ef1..96592f1956e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -17643,7 +17643,8 @@ ATSetTableSpaceCopyIndexes(AlterTableUtilityContext 
*context)
 {
        if (context == NULL || IsInTransactionBlock(context->isTopLevel))
                return true;
-       if (EventCacheLookup(EVT_DDLCommandEnd) != NIL)
+       if (EventCacheLookup(EVT_DDLCommandEnd) != NIL ||
+               EventCacheLookup(EVT_SQLDrop) != NIL)
                return true;
 
        MyXactFlags |= XACT_FLAGS_NEEDIMMEDIATECOMMIT;