From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Geoghegan <pg@bowt.ie>
cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
        PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Fixing findDependentObjects()'s dependency on scan order
 (regressions in DROP diagnostic messages)
In-reply-to: <15190.1547757720@sss.pgh.pa.us>
References: 
 <CAH2-WzmYbkT6e9B1jWApV5HcApUZqC==yoC4VTi7TBs007Ki+g@mail.gmail.com>
 <20181218212053.4vqcmmcicbzr2jyc@alvherre.pgsql>
 <CAH2-WzkMmRkXsWaVZ+VEbKdcKRcTuxu5ogVVH1SARqVuTuQKHA@mail.gmail.com>
 <21925.1545171102@sss.pgh.pa.us>
 <CAH2-Wz=rmUGgdp8K-FB1VRQngHk7MdtKvEATFQ_pP3_N257t8g@mail.gmail.com>
 <22791.1545171984@sss.pgh.pa.us>
 <CAH2-WzmQuksC_sWR8Z0aqxAVC6bBAcwU3bHv=rbf8Hp1RdJjvA@mail.gmail.com>
 <15190.1547757720@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Thu, 17 Jan 2019 15:42:00 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <15874.1547762019.0@sss.pgh.pa.us>
Date: Thu, 17 Jan 2019 16:54:36 -0500
Message-ID: <15908.1547762076@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <15874.1547762019.1@sss.pgh.pa.us>

I wrote:
> I propose that we handle this case by adding a new DEPFLAG_IS_SUBOBJECT
> flag to the column object's flags, denoting that we know the whole table
> will be dropped later.  The only effect of this flag is to suppress
> reporting of the column object in reportDependentObjects.

Here's a proposed patch for that bit.  As expected, it seems to eliminate
the variation in number-of-cascaded-drops-reported under
ignore_system_indexes.  I do not see any regression outputs change
otherwise.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="dont-report-subobjects-duplicatively-1.patch";
	charset="us-ascii"
Content-ID: <15874.1547762019.2@sss.pgh.pa.us>
Content-Description: dont-report-subobjects-duplicatively-1.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/depend=
ency.c
index 3529084..6b8c735 100644
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
*************** typedef struct
*** 102,107 ****
--- 102,108 ----
  #define DEPFLAG_INTERNAL	0x0008	/* reached via internal dependency */
  #define DEPFLAG_EXTENSION	0x0010	/* reached via extension dependency */
  #define DEPFLAG_REVERSE		0x0020	/* reverse internal/extension link */
+ #define DEPFLAG_SUBOBJECT	0x0040	/* subobject of another deletable objec=
t */
  =

  =

  /* expansible list of ObjectAddresses */
*************** reportDependentObjects(const ObjectAddre
*** 886,891 ****
--- 887,896 ----
  		if (extra->flags & DEPFLAG_ORIGINAL)
  			continue;
  =

+ 		/* Also ignore sub-objects; we'll report the whole object elsewhere */
+ 		if (extra->flags & DEPFLAG_SUBOBJECT)
+ 			continue;
+ =

  		objDesc =3D getObjectDescription(obj);
  =

  		/*
*************** object_address_present_add_flags(const O
*** 2320,2332 ****
  				 * DROP COLUMN action even though we know we're gonna delete
  				 * the table later.
  				 *
  				 * Because there could be other subobjects of this object in
  				 * the array, this case means we always have to loop through
  				 * the whole array; we cannot exit early on a match.
  				 */
  				ObjectAddressExtra *thisextra =3D addrs->extras + i;
  =

! 				thisextra->flags |=3D flags;
  			}
  		}
  	}
--- 2325,2343 ----
  				 * DROP COLUMN action even though we know we're gonna delete
  				 * the table later.
  				 *
+ 				 * What we can do, though, is mark this as a subobject so that
+ 				 * we don't report it separately, which is confusing because
+ 				 * it's unpredictable whether it happens or not.  But do so
+ 				 * only if flags !=3D 0 (flags =3D=3D 0 is a read-only probe).
+ 				 *
  				 * Because there could be other subobjects of this object in
  				 * the array, this case means we always have to loop through
  				 * the whole array; we cannot exit early on a match.
  				 */
  				ObjectAddressExtra *thisextra =3D addrs->extras + i;
  =

! 				if (flags)
! 					thisextra->flags |=3D (flags | DEPFLAG_SUBOBJECT);
  			}
  		}
  	}
*************** stack_address_present_add_flags(const Ob
*** 2374,2380 ****
  				 * object_address_present_add_flags(), we should propagate
  				 * flags for the whole object to each of its subobjects.
  				 */
! 				stackptr->flags |=3D flags;
  			}
  		}
  	}
--- 2385,2392 ----
  				 * object_address_present_add_flags(), we should propagate
  				 * flags for the whole object to each of its subobjects.
  				 */
! 				if (flags)
! 					stackptr->flags |=3D (flags | DEPFLAG_SUBOBJECT);
  			}
  		}
  	}

------- =_aaaaaaaaaa0--