Re: Change ereport level for QueuePartitionConstraintValidation

Sergei Kornilov <sk(at)zsrv(dot)org> writes:
> Thank you! It seems the most appropriate option for this test is to change @contrib_excludes
> Done in attached patch, will check appveyor reaction.

Appveyor seems happy, so I took a look through this. There's little
to say about 0001: if we're going to drop the elevel, that's what
results. 0002 is slightly more interesting. I did not like calling
the module "alter_table": we should encourage any future similar needs
to add more tests in this same directory, not invent whole new ones.
I went with "test_misc", but that's still open for bikeshedding of course.
I did a minor amount of cleanup in the test script, including running
it through pgperltidy, but no substantive changes. Also added a README.

I think there are basically two objections that might be raised to
committing this:

1. Making this a src/test/modules/ subdirectory, when there is no
actual extension module in it, is a triumph of expediency over
good file-tree structure. If there were no other constraints
I'd want to call it src/test/misc/ or src/test/tap/ or something
like that. The expediency angle is that if we do that, the
buildfarm client script will need changes to know about it.
Is it better to go with the long-term view and accept that we
won't have full buildfarm coverage right away?

2. It seems kind of expensive and redundant to duplicate all these
test cases from the core tests. On my machine the new test script
took close to 2.5 seconds as-submitted. I was able to knock that
down to 2.1 by the expedient of combining adjacent psql invocations
that we didn't need to examine the results of. But it still is
adding a noticeable amount of time to check-world, which takes only
circa 100s overall (with parallelism). Should we think about
deleting some of these test cases from the core tests?

(An argument not to do so is that the test conditions are a bit
different: since the TAP test starts a new session for each
query, it fails to exercise carry-over of relcache entries,
which might possibly be interesting in this area.)

Or, of course, we could forget the whole thing and switch the output
level for these messages to NOTICE instead. I'm not for that, but
now that we see what it'll cost us to have them better hidden, we can
at least have an informed debate.

Thoughts?

regards, tom lane