Re: In pageinspect, perform clean-up after testing gin-related functions

On Wed, Jul 11, 2018 at 11:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
>> On 2018-07-11 12:56:49 +0530, Amit Kapila wrote:
>>> Yeah, it is good practice to drop the objects at the end. It is
>>> strange that original commit adfb81d9e1 has this at the end of the
>>> test, but a later commit 367b99bbb1 by Tom has removed the Drop
>>> statement. AFAICS, this is just a silly mistake, but I might be
>>> missing something. Tom, do you remember any reason for doing so? If
>>> not, then I think we can revert back that change (aka commit Kuntal's
>>> patch).
>
>> We actually sometimes intentionally want to persist objects past the end
>> of the test. Allows to test pg_dump / pg_upgrade. Don't know whether
>> that's the case here, but it's worthwhile to note.
>
> I don't think our pg_dump testbed makes any use of contrib regression
> tests, so that's not the reason here. I believe I took out the DROP
> because it made it impossible to do additional manual tests after the end
> of an installcheck run without laboriously re-creating the test table.
>

Fair point, but using a generic name like 'test1' and leaving it can
sometimes cause confusion. In this case, it was not clear by looking
at the test and all the nearby tests (brin, btree and page) uses the
same table name and drops the table at end of the test. The name
conflict doesn't arise because the test for 'gin' was at the end of
those.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com