Fix catalog cleanup on DROP EXTERNAL TABLE

When dropping an external table, the pg_exttable catalog entry must
be removed as well to avoid dangling entries building up. Reverse
the logic in deleting the catalog entry to make sure we delete it
before scanning for a potential next one. Also fix up the tests for
this as they were testing the removal of the pg_class catalog entry
rather than the pg_exttable entry.

Reported by Ashwin Agrawal in Github issue #1498

src/backend/catalog/pg_exttable.c

@@ -474,8 +474,11 @@ RemoveExtTableEntry(Oid relid)
 	/*
 	 * Delete the external table entry from the catalog (pg_exttable).
 	 */
-	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
+	do
+	{
 		simple_heap_delete(pg_exttable_rel, &tuple->t_self);
+	}
+	while (HeapTupleIsValid(tuple = systable_getnext(scan)));
 
 	/* Finish up scan and close exttable catalog. */
 	systable_endscan(scan);

src/test/regress/input/external_table.source

@@ -232,22 +232,22 @@ drop external table check_ps;
 drop external table check_env;
 drop external table wet_pos4;
 
--- Tests for MPP-2513: dropping an external table must result in the deletion
--- of the relation pg_exttable row. This was done when using drop external
--- table but not drop schema cascade;
-
-create schema exttabletest;
-CREATE EXTERNAL TABLE exttabletest.EXT_NATION  ( N_NATIONKEY  INTEGER ,
-                            N_NAME       CHAR(25) ,
-                            N_REGIONKEY  INTEGER ,
-                            N_COMMENT    VARCHAR(152))
-location ('file://@hostname@@abs_srcdir@/data/nation.tbl' )
-FORMAT 'text' (delimiter '|');
--- Don't just return the row, as we'll get skew from OID differences between
--- runs.
-select count(*) from pg_catalog.pg_exttable where reloid in (select r.oid from pg_class r where r.relname in ('ext_nation', 'ext_whois', 'exttab1_gpfdist_status'));
-drop schema exttabletest cascade;
-select count(*) from pg_catalog.pg_exttable where reloid in (select r.oid from pg_class r where r.relname in ('ext_nation', 'ext_whois', 'exttab1_gpfdist_status'));
+-- Dropping an external table, both via DROP EXTERNAL TABLE and as a result
+-- from a cascading deletion must result in the deletion of the relation
+-- pg_exttable row.
+
+CREATE EXTERNAL WEB TABLE cascadetest (x text) EXECUTE 'echo "cascadetest";' FORMAT 'text';
+SELECT count(*) FROM pg_exttable WHERE command = 'echo "cascadetest";';
+DROP EXTERNAL TABLE cascadetest;
+SELECT count(*) FROM pg_exttable WHERE command = 'echo "cascadetest";';
+
+CREATE SCHEMA exttabletest;
+CREATE
+	EXTERNAL TABLE exttabletest.ext_nation (n_nationkey integer, n_name char(25), n_regionkey integer, n_comment varchar(152))
+	LOCATION ('file://@hostname@@abs_srcdir@/data/nation.tbl') FORMAT 'text' (delimiter '|');
+SELECT count(*) FROM pg_exttable WHERE location @> '{file://@hostname@@abs_srcdir@/data/nation.tbl}';
+DROP SCHEMA exttabletest CASCADE;
+SELECT count(*) FROM pg_exttable WHERE location @> '{file://@hostname@@abs_srcdir@/data/nation.tbl}';
 
 -- Tests for MPP17980: generating artificial CTIDs for external table scans
 -- This is necessary because the planner currently generates plans that include

src/test/regress/output/external_table.source

@@ -366,27 +366,36 @@ DROP ROLE exttab1_u6;
 drop external table check_ps;
 drop external table check_env;
 drop external table wet_pos4;
--- Tests for MPP-2513: dropping an external table must result in the deletion
--- of the relation pg_exttable row. This was done when using drop external
--- table but not drop schema cascade;
-create schema exttabletest;
-CREATE EXTERNAL TABLE exttabletest.EXT_NATION  ( N_NATIONKEY  INTEGER ,
-                            N_NAME       CHAR(25) ,
-                            N_REGIONKEY  INTEGER ,
-                            N_COMMENT    VARCHAR(152))
-location ('file://@hostname@@abs_srcdir@/data/nation.tbl' )
-FORMAT 'text' (delimiter '|');
--- Don't just return the row, as we'll get skew from OID differences between
--- runs.
-select count(*) from pg_catalog.pg_exttable where reloid in (select r.oid from pg_class r where r.relname in ('ext_nation', 'ext_whois', 'exttab1_gpfdist_status'));
+-- Dropping an external table, both via DROP EXTERNAL TABLE and as a result
+-- from a cascading deletion must result in the deletion of the relation
+-- pg_exttable row.
+CREATE EXTERNAL WEB TABLE cascadetest (x text) EXECUTE 'echo "cascadetest";' FORMAT 'text';
+SELECT count(*) FROM pg_exttable WHERE command = 'echo "cascadetest";';
+ count 
+-------
+     1
+(1 row)
+
+DROP EXTERNAL TABLE cascadetest;
+SELECT count(*) FROM pg_exttable WHERE command = 'echo "cascadetest";';
+ count 
+-------
+     0
+(1 row)
+
+CREATE SCHEMA exttabletest;
+CREATE
+	EXTERNAL TABLE exttabletest.ext_nation (n_nationkey integer, n_name char(25), n_regionkey integer, n_comment varchar(152))
+	LOCATION ('file://@hostname@@abs_srcdir@/data/nation.tbl') FORMAT 'text' (delimiter '|');
+SELECT count(*) FROM pg_exttable WHERE location @> '{file://@hostname@@abs_srcdir@/data/nation.tbl}';
  count 
 -------
      1
 (1 row)
 
-drop schema exttabletest cascade;
+DROP SCHEMA exttabletest CASCADE;
 NOTICE:  drop cascades to external table exttabletest.ext_nation
-select count(*) from pg_catalog.pg_exttable where reloid in (select r.oid from pg_class r where r.relname in ('ext_nation', 'ext_whois', 'exttab1_gpfdist_status'));
+SELECT count(*) FROM pg_exttable WHERE location @> '{file://@hostname@@abs_srcdir@/data/nation.tbl}';
  count 
 -------
      0