Re: [HACKERS] Regression tests vs existing users in an installation

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Regression tests vs existing users in an installation
Date: 2019-06-26 03:33:22
Message-ID: 1382.1561520002@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

[ blast from the past department ]

So, this thread about ensuring the regression tests don't create random
globally-visible names seems to have got lost in the weeds. I'm going
to resurrect it after noticing that two different places have grown
violations of the rule since I fixed things in 18555b132.

I think we were largely overdesigning the fix. The right thing is to do
something approximately as attached, and then make at least one buildfarm
critter build with -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS.

This proposed patch intentionally emits only WARNINGs, not ERRORS. This
is so that TAP tests won't fail if the warnings fire. Since TAP tests
never run against an existing installation, there's no reason for them
to follow the restriction. Admittedly, this is a pretty ad-hoc way of
getting that end result, but I'm tired of waiting for somebody to
implement a less ad-hoc way.

There are still two things we'd have to argue about before committing
this. One is the already-discussed-upthread point that rolenames.sql
insists on creating and then deleting users with names like "user".
I remain of the opinion that that's just a damfool idea; there is nearly
zero chance that those test cases will ever catch a bug, and much more
than zero chance that they'll cause problems in some pre-existing
installation. So my vote is to take them out.

The other is that we've also grown a bunch of tests that create
subscriptions and replication origins with randomly-chosen names.
Since those objects also have globally-visible names, I think the
"name them regress_something" policy should apply to them too, and
the attached patch tries to enforce it. But of course that causes
a bunch of failures right now.

(While I'm looking at that, I wonder why we don't have a restriction
against "pg_xxx" names for those object types.)

Comments?

regards, tom lane

Attachment Content-Type Size
enforce-rules-about-regression-global-object-names.patch text/x-diff 3.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-06-26 03:52:28 mcvstats serialization code is still shy of a load
Previous Message Thomas Munro 2019-06-26 03:07:49 Re: GiST VACUUM