pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
11+ messages / 3 participants
[nested] [flat]
* pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 01:18 Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 11+ messages in thread
From: Andres Freund @ 2018-11-27 01:18 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Update pg_upgrade test for reg* to include regrole and regnamespace.
When the regrole (0c90f6769) and regnamespace (cb9fa802b) types were
added in 9.5, pg_upgrade's check for reg* types wasn't updated. While
regrole currently is safe, regnamespace is not.
It seems unlikely that anybody uses regnamespace inside catalog tables
across a pg_upgrade, but the tests should be correct nevertheless.
While at it, reorder the types checked in the query to be
alphabetical. Otherwise it's annoying to compare existing and tested
for types.
Author: Andres Freund
Discussion: https://postgr.es/m/037e152a-cb25-3bcb-4f35-bdc9988f8204@2ndQuadrant.com
Backpatch: 9.5-, as regrole/regnamespace
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/ac218aa4f6dff6dfb4bf2675b2d9f2f23da6a7c5
Modified Files
--------------
src/bin/pg_upgrade/check.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 01:18 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 11+ messages in thread
From: Andres Freund @ 2018-11-27 01:18 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Update pg_upgrade test for reg* to include regrole and regnamespace.
When the regrole (0c90f6769) and regnamespace (cb9fa802b) types were
added in 9.5, pg_upgrade's check for reg* types wasn't updated. While
regrole currently is safe, regnamespace is not.
It seems unlikely that anybody uses regnamespace inside catalog tables
across a pg_upgrade, but the tests should be correct nevertheless.
While at it, reorder the types checked in the query to be
alphabetical. Otherwise it's annoying to compare existing and tested
for types.
Author: Andres Freund
Discussion: https://postgr.es/m/037e152a-cb25-3bcb-4f35-bdc9988f8204@2ndQuadrant.com
Backpatch: 9.5-, as regrole/regnamespace
Branch
------
REL_11_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/203a909fd29656d65a451c791e326ff92a09d893
Modified Files
--------------
src/bin/pg_upgrade/check.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 01:18 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 11+ messages in thread
From: Andres Freund @ 2018-11-27 01:18 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Update pg_upgrade test for reg* to include regrole and regnamespace.
When the regrole (0c90f6769) and regnamespace (cb9fa802b) types were
added in 9.5, pg_upgrade's check for reg* types wasn't updated. While
regrole currently is safe, regnamespace is not.
It seems unlikely that anybody uses regnamespace inside catalog tables
across a pg_upgrade, but the tests should be correct nevertheless.
While at it, reorder the types checked in the query to be
alphabetical. Otherwise it's annoying to compare existing and tested
for types.
Author: Andres Freund
Discussion: https://postgr.es/m/037e152a-cb25-3bcb-4f35-bdc9988f8204@2ndQuadrant.com
Backpatch: 9.5-, as regrole/regnamespace
Branch
------
REL_10_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/181d35af7bb074023156c802a6ae51208fcd7359
Modified Files
--------------
src/bin/pg_upgrade/check.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 01:18 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 11+ messages in thread
From: Andres Freund @ 2018-11-27 01:18 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Update pg_upgrade test for reg* to include regrole and regnamespace.
When the regrole (0c90f6769) and regnamespace (cb9fa802b) types were
added in 9.5, pg_upgrade's check for reg* types wasn't updated. While
regrole currently is safe, regnamespace is not.
It seems unlikely that anybody uses regnamespace inside catalog tables
across a pg_upgrade, but the tests should be correct nevertheless.
While at it, reorder the types checked in the query to be
alphabetical. Otherwise it's annoying to compare existing and tested
for types.
Author: Andres Freund
Discussion: https://postgr.es/m/037e152a-cb25-3bcb-4f35-bdc9988f8204@2ndQuadrant.com
Backpatch: 9.5-, as regrole/regnamespace
Branch
------
REL9_6_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/17024c08e8e07bebdb02917304c4aa2a9004d917
Modified Files
--------------
src/bin/pg_upgrade/check.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 01:18 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 11+ messages in thread
From: Andres Freund @ 2018-11-27 01:18 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Update pg_upgrade test for reg* to include regrole and regnamespace.
When the regrole (0c90f6769) and regnamespace (cb9fa802b) types were
added in 9.5, pg_upgrade's check for reg* types wasn't updated. While
regrole currently is safe, regnamespace is not.
It seems unlikely that anybody uses regnamespace inside catalog tables
across a pg_upgrade, but the tests should be correct nevertheless.
While at it, reorder the types checked in the query to be
alphabetical. Otherwise it's annoying to compare existing and tested
for types.
Author: Andres Freund
Discussion: https://postgr.es/m/037e152a-cb25-3bcb-4f35-bdc9988f8204@2ndQuadrant.com
Backpatch: 9.5-, as regrole/regnamespace
Branch
------
REL9_5_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/2bdca80fcc31b44b69628f7ba4b8a3b3ab23f1f1
Modified Files
--------------
src/bin/pg_upgrade/check.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 05:43 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2018-11-27 05:43 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-committers@lists.postgresql.org
Andres Freund <andres@anarazel.de> writes:
> Update pg_upgrade test for reg* to include regrole and regnamespace.
This has broken all the back-branch cross-version upgrade tests,
to go along with the HEAD cases you already broke :-(
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 06:32 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: Andres Freund @ 2018-11-27 06:32 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
Hi,
On 2018-11-27 00:43:43 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Update pg_upgrade test for reg* to include regrole and regnamespace.
>
> This has broken all the back-branch cross-version upgrade tests,
Hm, darn it. Gotta rewrite the query to reference those new types
without the regtype casts (as to_regtype without errors doesn't exist
far enough back). Seems easier than having a version check for the
different types.
> to go along with the HEAD cases you already broke :-(
The cross-version bug around pg_largeobject_metadata ought to be fixed
now. I'm hoping for a response by Andrew about the ordering bit, you're
welcome to opine on that too.
I see that prairiedog
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prairiedog&dt=2018-11-27%2005%3A30%3A39
failed just it's last run, but I'm not clear whether that's just a
random failure, or something I can't don't recognize without further
information.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 15:32 Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Dunstan @ 2018-11-27 15:32 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On 11/27/18 1:32 AM, Andres Freund wrote:
>
> The cross-version bug around pg_largeobject_metadata ought to be fixed
> now. I'm hoping for a response by Andrew about the ordering bit, you're
> welcome to opine on that too.
>
> I see that prairiedog
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prairiedog&dt=2018-11-27%2005%3A30%3A39
> failed just it's last run, but I'm not clear whether that's just a
> random failure, or something I can't don't recognize without further
> information.
The report is that the dumps differ. My bet is that this is another
instance of the LO ordering issue. I can handle that in the
cross-version tests because they are completely under my control (see
<https://github.com/PGBuildFarm/client-code/commit/9ca42ac1783a8cf99c73b4f7c52bd05a6024669d;)
, but not in the builtin test, which has this:
if diff "$temp_root"/dump1.sql "$temp_root"/dump2.sql >/dev/null; then
echo PASSED
exit 0
else
echo "Files $temp_root/dump1.sql and $temp_root/dump2.sql differ"
echo "dumps were not identical"
exit 1
fi
So that's all we know. Maybe we should change the script to output the
diff, or at least some of it, say the first 1000 lines.
In any case, I think we will probably need to bite the bullet and have
pg_dump render LOs in OID order. Not sure if that needs to be always on,
or an option.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 15:47 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2018-11-27 15:47 UTC (permalink / raw)
To: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-committers@lists.postgresql.org
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> In any case, I think we will probably need to bite the bullet and have
> pg_dump render LOs in OID order.
+1. Looking at the relevant code:
/*
* Currently, we re-fetch all BLOB OIDs using a cursor. Consider scanning
* the already-in-memory dumpable objects instead...
*/
if (fout->remoteVersion >= 90000)
blobQry = "DECLARE bloboid CURSOR FOR SELECT oid FROM pg_largeobject_metadata";
else
blobQry = "DECLARE bloboid CURSOR FOR SELECT DISTINCT loid FROM pg_largeobject";
I suspect that the original expectation was that SELECT DISTINCT would
deliver the blob OIDs in sorted order, and nobody noticed when hash-based
DISTINCT broke that, and then the pg_largeobject_metadata patch figured
that it didn't need to preserve a guarantee that wasn't there.
We have taken pains in the past to ensure consistent dump ordering, and
it seems to me the lack of that here is just an oversight. It's kinda
surprising that it hasn't bit us before.
> Not sure if that needs to be always on,
> or an option.
We have no existing option that says "you can skip ordering my dump
consistently". I see no need for one here.
The easy fix is to add "ORDER BY 1" to both of the above queries.
If anyone is concerned about the performance of that, they could
go try to do what the comment suggests, but I'm not personally
excited about that.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 20:38 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: Andres Freund @ 2018-11-27 20:38 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>; pgsql-committers@lists.postgresql.org
Hi,
On 2018-11-27 10:47:29 -0500, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> > In any case, I think we will probably need to bite the bullet and have
> > pg_dump render LOs in OID order.
>
> +1. Looking at the relevant code:
>
> /*
> * Currently, we re-fetch all BLOB OIDs using a cursor. Consider scanning
> * the already-in-memory dumpable objects instead...
> */
> if (fout->remoteVersion >= 90000)
> blobQry = "DECLARE bloboid CURSOR FOR SELECT oid FROM pg_largeobject_metadata";
> else
> blobQry = "DECLARE bloboid CURSOR FOR SELECT DISTINCT loid FROM pg_largeobject";
Done. Thanks for chiming in.
Andrew, the <= 9.2 failure https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2018-11-27%2018%3A18%3A21
seems to be related to nabstime etc not being available anymore. We've
not backpatched
Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: REL_11_STABLE Release: REL_11_0 [184951a48] 2018-10-12 19:33:56 -0400
Branch: REL_10_STABLE Release: REL_10_6 [9320263ae] 2018-10-12 19:33:56 -0400
Branch: REL9_6_STABLE Release: REL9_6_11 [2ad422ce1] 2018-10-12 19:33:56 -0400
Branch: REL9_5_STABLE Release: REL9_5_15 [43cc4e49e] 2018-10-12 19:33:56 -0400
Branch: REL9_4_STABLE Release: REL9_4_20 [7b88c1ddd] 2018-10-12 19:33:57 -0400
Branch: REL9_3_STABLE Release: REL9_3_25 [fb583c80d] 2018-10-12 19:33:57 -0400
Remove abstime, reltime, tinterval tables from old regression databases.
that far. I've locally verified that upgrade works after
SELECT format('ALTER TABLE %s DROP COLUMN %s;', attrelid::regclass, attname) FROM pg_attribute WHERE attrelid > 16384 AND atttypid IN ('abstime'::regtype, 'reltime'::regtype, 'tinterval'::regtype)
\gexec
or something in that vein.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp
@ 2018-11-27 21:26 Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Dunstan @ 2018-11-27 21:26 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On 11/27/18 3:38 PM, Andres Freund wrote:
> Hi,
>
> On 2018-11-27 10:47:29 -0500, Tom Lane wrote:
>> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>>> In any case, I think we will probably need to bite the bullet and have
>>> pg_dump render LOs in OID order.
>> +1. Looking at the relevant code:
>>
>> /*
>> * Currently, we re-fetch all BLOB OIDs using a cursor. Consider scanning
>> * the already-in-memory dumpable objects instead...
>> */
>> if (fout->remoteVersion >= 90000)
>> blobQry = "DECLARE bloboid CURSOR FOR SELECT oid FROM pg_largeobject_metadata";
>> else
>> blobQry = "DECLARE bloboid CURSOR FOR SELECT DISTINCT loid FROM pg_largeobject";
> Done. Thanks for chiming in.
>
> Andrew, the <= 9.2 failure https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2018-11-27%2018%3A18%3A21
> seems to be related to nabstime etc not being available anymore. We've
> not backpatched
>
> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Branch: REL_11_STABLE Release: REL_11_0 [184951a48] 2018-10-12 19:33:56 -0400
> Branch: REL_10_STABLE Release: REL_10_6 [9320263ae] 2018-10-12 19:33:56 -0400
> Branch: REL9_6_STABLE Release: REL9_6_11 [2ad422ce1] 2018-10-12 19:33:56 -0400
> Branch: REL9_5_STABLE Release: REL9_5_15 [43cc4e49e] 2018-10-12 19:33:56 -0400
> Branch: REL9_4_STABLE Release: REL9_4_20 [7b88c1ddd] 2018-10-12 19:33:57 -0400
> Branch: REL9_3_STABLE Release: REL9_3_25 [fb583c80d] 2018-10-12 19:33:57 -0400
>
> Remove abstime, reltime, tinterval tables from old regression databases.
>
> that far. I've locally verified that upgrade works after
>
> SELECT format('ALTER TABLE %s DROP COLUMN %s;', attrelid::regclass, attname) FROM pg_attribute WHERE attrelid > 16384 AND atttypid IN ('abstime'::regtype, 'reltime'::regtype, 'tinterval'::regtype)
> \gexec
>
> or something in that vein.
>
Thanks.
I'm just dropping the tables for REL_12. There is no point in just
dropping the columns - they are one-column tables.
The reason you're seeing this is that given the discussion about
pg_upgrade support I wanted to make sure that we had coverage for live +
2 releases. Should be green the next time it reports.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2018-11-27 21:26 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 01:18 pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp Andres Freund <andres@anarazel.de>
2018-11-27 05:43 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-11-27 06:32 ` Andres Freund <andres@anarazel.de>
2018-11-27 15:32 ` Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
2018-11-27 15:47 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-11-27 20:38 ` Andres Freund <andres@anarazel.de>
2018-11-27 21:26 ` Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
2018-11-27 01:18 pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp Andres Freund <andres@anarazel.de>
2018-11-27 01:18 pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp Andres Freund <andres@anarazel.de>
2018-11-27 01:18 pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp Andres Freund <andres@anarazel.de>
2018-11-27 01:18 pgsql: Update pg_upgrade test for reg* to include regrole and regnamesp Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox