Обсуждение: pgsql: Remove pg_class.relhaspkey
Remove pg_class.relhaspkey It is not used for anything internally, and it cannot be relied on for external uses, so it can just be removed. To correct recommended way to check for a primary key is in pg_index. Discussion: https://www.postgresql.org/message-id/flat/b1a24c6c-6913-f89c-674e-0704f0ed69db@2ndquadrant.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/f66e8bf875f691db4c5d0173fc39b5a9c3ec969c Modified Files -------------- doc/src/sgml/catalogs.sgml | 9 --------- src/backend/catalog/heap.c | 1 - src/backend/catalog/index.c | 32 ++----------------------------- src/backend/commands/vacuum.c | 10 ---------- src/backend/rewrite/rewriteDefine.c | 1 - src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_class.h | 38 ++++++++++++++++++------------------- 7 files changed, 21 insertions(+), 72 deletions(-)
Hi, On 2018-03-14 19:35:40 +0000, Peter Eisentraut wrote: > Remove pg_class.relhaspkey > > It is not used for anything internally, and it cannot be relied on for > external uses, so it can just be removed. To correct recommended way to > check for a primary key is in pg_index. > > Discussion: https://www.postgresql.org/message-id/flat/b1a24c6c-6913-f89c-674e-0704f0ed69db@2ndquadrant.com I'm not clear on the mechanics, and the animal doesn't show the critical log. But this might have caused the issue, being the only commit between runs: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2018-03-14%2019%3A37%3A20 Andrew, any chance you could modify the module to capture pg_upgrade_dump_*.log? Greetings, Andres Freund
Greetings, * Andres Freund (andres@anarazel.de) wrote: > On 2018-03-14 19:35:40 +0000, Peter Eisentraut wrote: > > Remove pg_class.relhaspkey > > > > It is not used for anything internally, and it cannot be relied on for > > external uses, so it can just be removed. To correct recommended way to > > check for a primary key is in pg_index. > > > > Discussion: https://www.postgresql.org/message-id/flat/b1a24c6c-6913-f89c-674e-0704f0ed69db@2ndquadrant.com > > I'm not clear on the mechanics, and the animal doesn't show the critical > log. But this might have caused the issue, being the only commit between > runs: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2018-03-14%2019%3A37%3A20 > > Andrew, any chance you could modify the module to capture > pg_upgrade_dump_*.log? I've not quite tracked it down, but I would caution against blaming this commit- when doing some parallel regression test runs, I was seeing failures also, but they've not been consistent and I was trying to get something else done so didn't focus on them, so they may have been failures due to my environment, but might also be some kind of race condition in an earlier commit (my guess at the moment is actually the INOUT arguments for procedures commit...). I'll try doing some more runs to see if I can reproduce it, but wanted to mention it, just to encourage people to consider looking more broadly than just this commit if they run into similar issues. Thanks! Stephen
Вложения
On 2018-03-14 18:09:07 -0400, Stephen Frost wrote: > Greetings, > > * Andres Freund (andres@anarazel.de) wrote: > > On 2018-03-14 19:35:40 +0000, Peter Eisentraut wrote: > > > Remove pg_class.relhaspkey > > > > > > It is not used for anything internally, and it cannot be relied on for > > > external uses, so it can just be removed. To correct recommended way to > > > check for a primary key is in pg_index. > > > > > > Discussion: https://www.postgresql.org/message-id/flat/b1a24c6c-6913-f89c-674e-0704f0ed69db@2ndquadrant.com > > > > I'm not clear on the mechanics, and the animal doesn't show the critical > > log. But this might have caused the issue, being the only commit between > > runs: > > > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2018-03-14%2019%3A37%3A20 > > > > Andrew, any chance you could modify the module to capture > > pg_upgrade_dump_*.log? > > I've not quite tracked it down, but I would caution against blaming this > commit- when doing some parallel regression test runs, I was seeing > failures also, but they've not been consistent and I was trying to get > something else done so didn't focus on them, so they may have been > failures due to my environment, but might also be some kind of race > condition in an earlier commit (my guess at the moment is actually the > INOUT arguments for procedures commit...). > > I'll try doing some more runs to see if I can reproduce it, but wanted > to mention it, just to encourage people to consider looking more broadly > than just this commit if they run into similar issues. It failed identically twice in a row now, preceded by a significant number of successful runs: https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=crake&br=HEAD and afaik there's no concurrency during the cross-version test. So sure, it's possible that something else is to blame, but I'd not bet on it. Greetings, Andres Freund
On Thu, Mar 15, 2018 at 7:43 AM, Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2018-03-14 19:35:40 +0000, Peter Eisentraut wrote:
> Remove pg_class.relhaspkey
>
> It is not used for anything internally, and it cannot be relied on for
> external uses, so it can just be removed. To correct recommended way to
> check for a primary key is in pg_index.
>
> Discussion: https://www.postgresql.org/message-id/flat/b1a24c6c-6913- f89c-674e-0704f0ed69db@ 2ndquadrant.com
I'm not clear on the mechanics, and the animal doesn't show the critical
log. But this might have caused the issue, being the only commit between
runs:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm= crake&dt=2018-03-14%2019%3A37% 3A20
Andrew, any chance you could modify the module to capture
pg_upgrade_dump_*.log?
I will look at it. Meanwhile I have fixed a typo in the module. The database in question is supposed to have been dropped as it has caused other issues in the past, but due to the typo it wasn't.
Basing an MV on pg_class could always be difficult for pg_upgrade. Maybe that's not a brilliant thing to do in a test (or maybe the test should drop the MV after it's done).
cheers
andrew
Stephen Frost <sfrost@snowman.net> writes: > I've not quite tracked it down, but I would caution against blaming this > commit- when doing some parallel regression test runs, I was seeing > failures also, but they've not been consistent and I was trying to get > something else done so didn't focus on them, so they may have been > failures due to my environment, but might also be some kind of race > condition in an earlier commit (my guess at the moment is actually the > INOUT arguments for procedures commit...). crake has now failed twice at the same spot, so it's looking reproducible to me. I've enabled TestUpgradeXversion on longfin and will try to reproduce the problem there, if Andrew doesn't manage to scrape the relevant log out of crake first. (It'll probably take a few hours for me to get results.) The broader picture is that we've been having irreproducible failures on the buildfarm since around December. Some of them are in postgres_fdw, and I'd originally suspected a problem there, but we've also seen more than one select_parallel failure like this one: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=longfin&dt=2018-03-11%2023%3A25%3A46 My gut instinct right now is that those are related, since they both look like "plan change with no visible triggering cause". I have no ideas about a plausible explanation. regards, tom lane
Andrew Dunstan <andrew@dunslane.net> writes:
> Basing an MV on pg_class could always be difficult for pg_upgrade. Maybe
> that's not a brilliant thing to do in a test (or maybe the test should drop
> the MV after it's done).
OH. Is that what it's doing? The cause of the failure is immediately
obvious then. pg_class now lacks a relhaspkey column, but the matview
is still dependent on one being there.
I concur that this is not well-considered test design.
regards, tom lane
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > Basing an MV on pg_class could always be difficult for pg_upgrade. Maybe > > that's not a brilliant thing to do in a test (or maybe the test should drop > > the MV after it's done). > > OH. Is that what it's doing? The cause of the failure is immediately > obvious then. pg_class now lacks a relhaspkey column, but the matview > is still dependent on one being there. > > I concur that this is not well-considered test design. It seems very strange to me that the test_ddl_deparse database would be used by a pg_upgrade test, but OK. I'll change it. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 15, 2018 at 9:41 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > Tom Lane wrote: >> Andrew Dunstan <andrew@dunslane.net> writes: >> > Basing an MV on pg_class could always be difficult for pg_upgrade. Maybe >> > that's not a brilliant thing to do in a test (or maybe the test should drop >> > the MV after it's done). >> >> OH. Is that what it's doing? The cause of the failure is immediately >> obvious then. pg_class now lacks a relhaspkey column, but the matview >> is still dependent on one being there. >> >> I concur that this is not well-considered test design. > > It seems very strange to me that the test_ddl_deparse database would be > used by a pg_upgrade test, but OK. I'll change it. > As I mentioned upthread, it's not supposed to be, but was being due to a typo that I have fixed. You should see this error cleared on the dashboard in a few minutes. However, in general the module tries to do a maximal test. That includes almost all the contrib databases. That approach has helped reveal problems several times in the past. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan wrote: > As I mentioned upthread, it's not supposed to be, but was being due to > a typo that I have fixed. You should see this error cleared on the > dashboard in a few minutes. > > However, in general the module tries to do a maximal test. That > includes almost all the contrib databases. That approach has helped > reveal problems several times in the past. Well, maybe we should dictate policy that it must be possible to pg_upgrade this database too; then we'll just have to work so that it always works. I have fixed the current problem and I'm open to the idea of keeping it working. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Well, maybe we should dictate policy that it must be possible to
> pg_upgrade this database too; then we'll just have to work so that it
> always works. I have fixed the current problem and I'm open to the idea
> of keeping it working.
Name of the matview now seems a bit confusing ...
regards, tom lane