From: | Georgios <gkokolatos(at)protonmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Justin Pryzby <pryzby(at)telsasoft(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Include access method in listTables output |
Date: | 2020-09-01 10:27:31 |
Message-ID: | nqWijaisx2TwPSOzei5ebun5AwLl1xWkjezNZn4OASe_Pg9oSp5h0mL57tsIzV01DnxHu0Tym7OvCcLA9AnlFISN9012kd_1cJA1o6NH664=@protonmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, 1 September 2020 07:41, Michael Paquier <michael(at)paquier(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Alexey Kondratov | 2020-09-01 10:36:38 | Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly |
Previous Message | Anastasia Lubennikova | 2020-09-01 10:25:27 | Re: REINDEX SCHEMA/DATABASE/SYSTEM weak with dropped relations |