‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 1 September 2020 07:41, Michael Paquier <mich...@paquier.xyz> wrote:

> On Thu, Aug 20, 2020 at 08:16:19AM +0000, Georgios wrote:
>
> > Please find version 7 attached which hopefully addresses the error along 
> > with a proper
> > expansion of the test coverage and removal of recently introduced
> > whitespace warnings.
>
> +CREATE ROLE conditional_tableam_display_role;
> As a convention, regression tests need to have roles prefixed with
> "regress_" or this would cause some buildfarm members to turn red.
> Please see -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS (you could use
> that in your environment for example).


I was wondering about the name. I hoped that it would either come up during 
review, or that it would be fine.

Thank you for the detailed explanation.

>
> So, as of the tests.. The role gets added to make sure that when
> using \d+ on the full schema as well as the various \d*+ variants we
> have a consistent owner. The addition of the relation size for the
> sequence and the btree index in the output generated is a problem
> though, because that's not really portable when compiling with other
> page sizes. It is true that there are other tests failing in this
> case, but I think that we should try to limit that if we can. In
> short, I agree that having some tests is better than nothing, but I
> would suggest to reduce their scope, as per the attached.

I could not agree more. I have only succumbed to the pressure of reviewing.

>
> Adding \dE as there are no foreign tables does not make much sense,
> and also I wondered why \dt+ was not added.
>
> Does the attached look correct to you?

You have my :+1:

>
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Michael




Reply via email to