pg_dump bug for extension owned tables
Found when working against release 12.
Given the following extension:
::::::::::::::
share/postgresql/extension/dummy--1.0.0.sql
::::::::::::::
-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "CREATE EXTENSION dummy" to load this file. \quit
CREATE TABLE @extschema@.dummytab (
a int,
b int,
c int);
SELECT pg_catalog.pg_extension_config_dump('dummytab', '');
::::::::::::::
share/postgresql/extension/dummy.control
::::::::::::::
# dummy extension
comment = 'dummy'
default_version = '1.0.0'
relocatable = false
and this use of it:
bin/psql -c 'create schema dummy; create extension dummy schema dummy;
insert into dummy.dummytab values(1,2,3);'
this command segfaults:
bin/pg_dump -a --column-inserts -n dummy
It appears that for extension owned tables tbinfo.attgenerated isn't
being properly populated, so line 2050 in REL_12_STABLE, which is line
2109 in git tip, is failing.
I'm looking for a fix, but if anyone has a quick fix that would be nice :-)
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables tbinfo.attgenerated isn't
being properly populated, so line 2050 in REL_12_STABLE, which is line
2109 in git tip, is failing.
Should have mentioned this is in src/bin/pg_dump/pg_dump.c
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Jun 26, 2020 at 11:24 AM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:
On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables tbinfo.attgenerated isn't
being properly populated, so line 2050 in REL_12_STABLE, which is line
2109 in git tip, is failing.Should have mentioned this is in src/bin/pg_dump/pg_dump.c
Having a look on it.
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
On Fri, Jun 26, 2020 at 11:55 AM Fabrízio de Royes Mello <
fabriziomello@gmail.com> wrote:
On Fri, Jun 26, 2020 at 11:24 AM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:
On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables tbinfo.attgenerated isn't
being properly populated, so line 2050 in REL_12_STABLE, which is line
2109 in git tip, is failing.Should have mentioned this is in src/bin/pg_dump/pg_dump.c
Having a look on it.
Seems when qualify the schemaname the the "tbinfo->interesting" field is
not setted for extensions objects, so the getTableAttrs can't fill the
attgenerated field properly.
I'm not 100% sure it's the correct way but the attached patch works for me
and all tests passed. Maybe we should add more TAP tests?
Regards,
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Attachments:
pg_dump_segfault_on_extension_and_schema_v1.patchtext/x-patch; charset=US-ASCII; name=pg_dump_segfault_on_extension_and_schema_v1.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a41a3db876..e12811832f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -18063,6 +18063,9 @@ processExtensionTables(Archive *fout, ExtensionInfo extinfo[],
if (strlen(extconditionarray[j]) > 0)
configtbl->dataObj->filtercond = pg_strdup(extconditionarray[j]);
}
+
+ if (configtbl->dobj.dump != DUMP_COMPONENT_NONE)
+ configtbl->interesting = true;
}
}
}
On 6/26/20 2:10 PM, Fabrízio de Royes Mello wrote:
On Fri, Jun 26, 2020 at 11:55 AM Fabrízio de Royes Mello
<fabriziomello@gmail.com <mailto:fabriziomello@gmail.com>> wrote:On Fri, Jun 26, 2020 at 11:24 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>> wrote:On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables tbinfo.attgenerated isn't
being properly populated, so line 2050 in REL_12_STABLE, whichis line
2109 in git tip, is failing.
Should have mentioned this is in src/bin/pg_dump/pg_dump.c
Having a look on it.
Seems when qualify the schemaname the the "tbinfo->interesting" field
is not setted for extensions objects, so the getTableAttrs can't fill
the attgenerated field properly.I'm not 100% sure it's the correct way but the attached patch works
for me and all tests passed. Maybe we should add more TAP tests?
Thanks for this.
It looks sane to me, I'll let others weigh in on it though. Yes we
should have a TAP test for it.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 6/26/20 2:10 PM, Fabrízio de Royes Mello wrote:
On Fri, Jun 26, 2020 at 11:55 AM Fabrízio de Royes Mello
<fabriziomello@gmail.com <mailto:fabriziomello@gmail.com>> wrote:On Fri, Jun 26, 2020 at 11:24 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>> wrote:On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables tbinfo.attgenerated isn't
being properly populated, so line 2050 in REL_12_STABLE, whichis line
2109 in git tip, is failing.
Should have mentioned this is in src/bin/pg_dump/pg_dump.c
Having a look on it.
Seems when qualify the schemaname the the "tbinfo->interesting" field
is not setted for extensions objects, so the getTableAttrs can't fill
the attgenerated field properly.I'm not 100% sure it's the correct way but the attached patch works
for me and all tests passed. Maybe we should add more TAP tests?
I just tried this patch out on master, with the test case I gave
upthread. It's not working, still getting a segfault.
Will look closer.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sat, Jul 11, 2020 at 8:07 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:
On 6/26/20 2:10 PM, Fabrízio de Royes Mello wrote:
On Fri, Jun 26, 2020 at 11:55 AM Fabrízio de Royes Mello
<fabriziomello@gmail.com <mailto:fabriziomello@gmail.com>> wrote:On Fri, Jun 26, 2020 at 11:24 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>> wrote:On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables tbinfo.attgenerated
isn't
being properly populated, so line 2050 in REL_12_STABLE, which
is line
2109 in git tip, is failing.
Should have mentioned this is in src/bin/pg_dump/pg_dump.c
Having a look on it.
Seems when qualify the schemaname the the "tbinfo->interesting" field
is not setted for extensions objects, so the getTableAttrs can't fill
the attgenerated field properly.I'm not 100% sure it's the correct way but the attached patch works
for me and all tests passed. Maybe we should add more TAP tests?I just tried this patch out on master, with the test case I gave
upthread. It's not working, still getting a segfault.
Ohh really sorry about it... my bad... i completely forgot about it!!!
Due to my rush I ended up adding the wrong patch version. Attached the
correct version.
Will add TAP tests to src/test/modules/test_pg_dump
Regards,
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Attachments:
pg_dump_segfault_on_extension_and_schema_v2.patchtext/x-patch; charset=US-ASCII; name=pg_dump_segfault_on_extension_and_schema_v2.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e758b5c50a..a998e5c821 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -18037,6 +18037,8 @@ processExtensionTables(Archive *fout, ExtensionInfo extinfo[],
configtbl->dataObj->filtercond = pg_strdup(extconditionarray[j]);
}
}
+
+ configtbl->interesting = dumpobj;
}
}
if (extconfigarray)
On Mon, Jul 13, 2020 at 11:52 AM Fabrízio de Royes Mello <
fabriziomello@gmail.com> wrote:
On Sat, Jul 11, 2020 at 8:07 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:
On 6/26/20 2:10 PM, Fabrízio de Royes Mello wrote:
On Fri, Jun 26, 2020 at 11:55 AM Fabrízio de Royes Mello
<fabriziomello@gmail.com <mailto:fabriziomello@gmail.com>> wrote:On Fri, Jun 26, 2020 at 11:24 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>> wrote:On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables tbinfo.attgenerated
isn't
being properly populated, so line 2050 in REL_12_STABLE, which
is line
2109 in git tip, is failing.
Should have mentioned this is in src/bin/pg_dump/pg_dump.c
Having a look on it.
Seems when qualify the schemaname the the "tbinfo->interesting" field
is not setted for extensions objects, so the getTableAttrs can't fill
the attgenerated field properly.I'm not 100% sure it's the correct way but the attached patch works
for me and all tests passed. Maybe we should add more TAP tests?I just tried this patch out on master, with the test case I gave
upthread. It's not working, still getting a segfault.Ohh really sorry about it... my bad... i completely forgot about it!!!
Due to my rush I ended up adding the wrong patch version. Attached the
correct version.
Will add TAP tests to src/test/modules/test_pg_dump
Attached the patch including TAP tests.
Regards,
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Attachments:
pg_dump_segfault_on_extension_and_schema_v3.patchtext/x-patch; charset=US-ASCII; name=pg_dump_segfault_on_extension_and_schema_v3.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e758b5c50a..a998e5c821 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -18037,6 +18037,8 @@ processExtensionTables(Archive *fout, ExtensionInfo extinfo[],
configtbl->dataObj->filtercond = pg_strdup(extconditionarray[j]);
}
}
+
+ configtbl->interesting = dumpobj;
}
}
if (extconfigarray)
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index ae120a5ee3..78aa07ce51 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -135,6 +135,12 @@ my %pgdump_runs = (
"$tempdir/defaults_tar_format.tar",
],
},
+ extension_schema => {
+ dump_cmd => [
+ 'pg_dump', '--schema=public', '--inserts',
+ "--file=$tempdir/extension_schema.sql", 'postgres',
+ ],
+ },
pg_dumpall_globals => {
dump_cmd => [
'pg_dumpall', '--no-sync',
@@ -301,8 +307,9 @@ my %tests = (
\n/xm,
like => {
%full_runs,
- data_only => 1,
- section_data => 1,
+ data_only => 1,
+ section_data => 1,
+ extension_schema => 1,
},
},
@@ -536,6 +543,7 @@ my %tests = (
like => {%pgdump_runs},
unlike => {
data_only => 1,
+ extension_schema => 1,
pg_dumpall_globals => 1,
section_data => 1,
section_pre_data => 1,
@@ -549,6 +557,7 @@ my %tests = (
like => {%pgdump_runs},
unlike => {
data_only => 1,
+ extension_schema => 1,
pg_dumpall_globals => 1,
section_data => 1,
section_pre_data => 1,
@@ -569,6 +578,17 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
+ },
+
+ # Dumpable object inside specific schema
+ 'INSERT INTO public.regress_table_dumpable VALUES (1);' => {
+ create_sql => 'INSERT INTO public.regress_table_dumpable VALUES (1);',
+ regexp => qr/^
+ \QINSERT INTO public.regress_table_dumpable VALUES (1);\E
+ \n/xm,
+ like => {
+ extension_schema => 1,
+ },
},);
#########################################
diff --git a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql b/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
index 3ed007a7b1..90e461ed35 100644
--- a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
+++ b/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
@@ -13,6 +13,11 @@ CREATE SEQUENCE regress_pg_dump_seq;
CREATE SEQUENCE regress_seq_dumpable;
SELECT pg_catalog.pg_extension_config_dump('regress_seq_dumpable', '');
+CREATE TABLE regress_table_dumpable (
+ col1 int
+);
+SELECT pg_catalog.pg_extension_config_dump('regress_table_dumpable', '');
+
CREATE SCHEMA regress_pg_dump_schema;
GRANT USAGE ON regress_pg_dump_seq TO regress_dump_test_role;
On 7/13/20 10:52 AM, Fabrízio de Royes Mello wrote:
On Sat, Jul 11, 2020 at 8:07 PM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>> wrote:On 6/26/20 2:10 PM, Fabrízio de Royes Mello wrote:
On Fri, Jun 26, 2020 at 11:55 AM Fabrízio de Royes Mello
<fabriziomello@gmail.com <mailto:fabriziomello@gmail.com><mailto:fabriziomello@gmail.com <mailto:fabriziomello@gmail.com>>> wrote:
On Fri, Jun 26, 2020 at 11:24 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>
<mailto:andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>>> wrote:
On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables
tbinfo.attgenerated isn't
being properly populated, so line 2050 in REL_12_STABLE, which
is line
2109 in git tip, is failing.
Should have mentioned this is in src/bin/pg_dump/pg_dump.c
Having a look on it.
Seems when qualify the schemaname the the "tbinfo->interesting" field
is not setted for extensions objects, so the getTableAttrs can't fill
the attgenerated field properly.I'm not 100% sure it's the correct way but the attached patch works
for me and all tests passed. Maybe we should add more TAP tests?I just tried this patch out on master, with the test case I gave
upthread. It's not working, still getting a segfault.Ohh really sorry about it... my bad... i completely forgot about it!!!
Due to my rush I ended up adding the wrong patch version. Attached the
correct version.Will add TAP tests to src/test/modules/test_pg_dump
yeah, that's the fix I came up with too. The only thing I added was
"Assert(tbinfo->attgenerated);" at about line 2097.
Will wait for your TAP tests.
thanks
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Jul 13, 2020 at 3:29 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:
On 7/13/20 10:52 AM, Fabrízio de Royes Mello wrote:
On Sat, Jul 11, 2020 at 8:07 PM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>> wrote:On 6/26/20 2:10 PM, Fabrízio de Royes Mello wrote:
On Fri, Jun 26, 2020 at 11:55 AM Fabrízio de Royes Mello
<fabriziomello@gmail.com <mailto:fabriziomello@gmail.com><mailto:fabriziomello@gmail.com <mailto:fabriziomello@gmail.com>>>
wrote:
On Fri, Jun 26, 2020 at 11:24 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>
<mailto:andrew.dunstan@2ndquadrant.com
<mailto:andrew.dunstan@2ndquadrant.com>>> wrote:
On 6/26/20 9:57 AM, Andrew Dunstan wrote:
It appears that for extension owned tables
tbinfo.attgenerated isn't
being properly populated, so line 2050 in REL_12_STABLE, which
is line
2109 in git tip, is failing.
Should have mentioned this is in src/bin/pg_dump/pg_dump.c
Having a look on it.
Seems when qualify the schemaname the the "tbinfo->interesting"
field
is not setted for extensions objects, so the getTableAttrs can't
fill
the attgenerated field properly.
I'm not 100% sure it's the correct way but the attached patch works
for me and all tests passed. Maybe we should add more TAP tests?I just tried this patch out on master, with the test case I gave
upthread. It's not working, still getting a segfault.Ohh really sorry about it... my bad... i completely forgot about it!!!
Due to my rush I ended up adding the wrong patch version. Attached the
correct version.Will add TAP tests to src/test/modules/test_pg_dump
yeah, that's the fix I came up with too. The only thing I added was
"Assert(tbinfo->attgenerated);" at about line 2097.
Cool.
Will wait for your TAP tests.
Actually I've sent it already but it seems to have gone to the moderation
queue.
Anyway attached with your assertion and TAP tests.
Regards,
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Attachments:
pg_dump_segfault_on_extension_and_schema_v4.patchtext/x-patch; charset=US-ASCII; name=pg_dump_segfault_on_extension_and_schema_v4.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e758b5c50a..baacd7af63 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2094,6 +2094,8 @@ dumpTableData_insert(Archive *fout, void *dcontext)
if (nfields == 0)
continue;
+ Assert(tbinfo->attgenerated);
+
/* Emit a row heading */
if (rows_per_statement == 1)
archputs(" (", fout);
@@ -18037,6 +18039,8 @@ processExtensionTables(Archive *fout, ExtensionInfo extinfo[],
configtbl->dataObj->filtercond = pg_strdup(extconditionarray[j]);
}
}
+
+ configtbl->interesting = dumpobj;
}
}
if (extconfigarray)
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index ae120a5ee3..78aa07ce51 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -135,6 +135,12 @@ my %pgdump_runs = (
"$tempdir/defaults_tar_format.tar",
],
},
+ extension_schema => {
+ dump_cmd => [
+ 'pg_dump', '--schema=public', '--inserts',
+ "--file=$tempdir/extension_schema.sql", 'postgres',
+ ],
+ },
pg_dumpall_globals => {
dump_cmd => [
'pg_dumpall', '--no-sync',
@@ -301,8 +307,9 @@ my %tests = (
\n/xm,
like => {
%full_runs,
- data_only => 1,
- section_data => 1,
+ data_only => 1,
+ section_data => 1,
+ extension_schema => 1,
},
},
@@ -536,6 +543,7 @@ my %tests = (
like => {%pgdump_runs},
unlike => {
data_only => 1,
+ extension_schema => 1,
pg_dumpall_globals => 1,
section_data => 1,
section_pre_data => 1,
@@ -549,6 +557,7 @@ my %tests = (
like => {%pgdump_runs},
unlike => {
data_only => 1,
+ extension_schema => 1,
pg_dumpall_globals => 1,
section_data => 1,
section_pre_data => 1,
@@ -569,6 +578,17 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
+ },
+
+ # Dumpable object inside specific schema
+ 'INSERT INTO public.regress_table_dumpable VALUES (1);' => {
+ create_sql => 'INSERT INTO public.regress_table_dumpable VALUES (1);',
+ regexp => qr/^
+ \QINSERT INTO public.regress_table_dumpable VALUES (1);\E
+ \n/xm,
+ like => {
+ extension_schema => 1,
+ },
},);
#########################################
diff --git a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql b/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
index 3ed007a7b1..90e461ed35 100644
--- a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
+++ b/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
@@ -13,6 +13,11 @@ CREATE SEQUENCE regress_pg_dump_seq;
CREATE SEQUENCE regress_seq_dumpable;
SELECT pg_catalog.pg_extension_config_dump('regress_seq_dumpable', '');
+CREATE TABLE regress_table_dumpable (
+ col1 int
+);
+SELECT pg_catalog.pg_extension_config_dump('regress_table_dumpable', '');
+
CREATE SCHEMA regress_pg_dump_schema;
GRANT USAGE ON regress_pg_dump_seq TO regress_dump_test_role;
On 7/13/20 2:46 PM, Fabrízio de Royes Mello wrote:
yeah, that's the fix I came up with too. The only thing I added was
"Assert(tbinfo->attgenerated);" at about line 2097.Cool.
Will wait for your TAP tests.
Actually I've sent it already but it seems to have gone to the
moderation queue.Anyway attached with your assertion and TAP tests.
Thanks, that all seems fine. The TAP test changes are a bit of a pain in
the neck before release 11, so I think I'll just do those back that far,
but the main fix for all live branches.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Jul 13, 2020 at 5:05 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:
On 7/13/20 2:46 PM, Fabrízio de Royes Mello wrote:
yeah, that's the fix I came up with too. The only thing I added was
"Assert(tbinfo->attgenerated);" at about line 2097.Cool.
Will wait for your TAP tests.
Actually I've sent it already but it seems to have gone to the
moderation queue.Anyway attached with your assertion and TAP tests.
Thanks, that all seems fine. The TAP test changes are a bit of a pain in
the neck before release 11, so I think I'll just do those back that far,
but the main fix for all live branches.
Sounds good to me.
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
On Mon, Jul 13, 2020 at 6:18 PM Fabrízio de Royes Mello <
fabriziomello@gmail.com> wrote:
On Mon, Jul 13, 2020 at 5:05 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:On 7/13/20 2:46 PM, Fabrízio de Royes Mello wrote:
yeah, that's the fix I came up with too. The only thing I added was
"Assert(tbinfo->attgenerated);" at about line 2097.Cool.
Will wait for your TAP tests.
Actually I've sent it already but it seems to have gone to the
moderation queue.Anyway attached with your assertion and TAP tests.
Thanks, that all seems fine. The TAP test changes are a bit of a pain in
the neck before release 11, so I think I'll just do those back that far,
but the main fix for all live branches.Sounds good to me.
Just added to the next commitfest [1]https://commitfest.postgresql.org/29/2671/ to make sure we'll not lose it.
Regards,
[1]: https://commitfest.postgresql.org/29/2671/
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
On Thu, Aug 6, 2020 at 4:12 PM Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
On Mon, Jul 13, 2020 at 6:18 PM Fabrízio de Royes Mello <fabriziomello@gmail.com> wrote:
On Mon, Jul 13, 2020 at 5:05 PM Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
On 7/13/20 2:46 PM, Fabrízio de Royes Mello wrote:
yeah, that's the fix I came up with too. The only thing I added was
"Assert(tbinfo->attgenerated);" at about line 2097.Cool.
Will wait for your TAP tests.
Actually I've sent it already but it seems to have gone to the
moderation queue.Anyway attached with your assertion and TAP tests.
Thanks, that all seems fine. The TAP test changes are a bit of a pain in
the neck before release 11, so I think I'll just do those back that far,
but the main fix for all live branches.Sounds good to me.
Just added to the next commitfest [1] to make sure we'll not lose it.
Regards,
Thanks, Committed. Further investigation shows this was introduced in
release 12, so that's how far back I went.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Sep 4, 2020 at 3:00 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:
Thanks, Committed. Further investigation shows this was introduced in
release 12, so that's how far back I went.
Thanks!
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
Thanks, Committed. Further investigation shows this was introduced in
release 12, so that's how far back I went.
Still further investigation shows that this patch caused bug #16655 [1]/messages/by-id/16655-5c92d6b3a9438137@postgresql.org.
It should *not* have been designed to unconditionally clear the
table's "interesting" flag, as there may have been other reasons
why that was set. The right way to think about it is "if we are
going to dump the table's data, then the table certainly needs its
interesting flag set, so that we'll collect the per-attribute info.
Otherwise leave well enough alone".
The patches I proposed in the other thread seem like they really ought
to go all the way back for safety's sake. However, I do not observe
any crash on the test case in v11, and I'm kind of wondering why not.
Did you identify exactly where this was "introduced in release 12"?
regards, tom lane
On 10/6/20 5:19 PM, Tom Lane wrote:
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
Thanks, Committed. Further investigation shows this was introduced in
release 12, so that's how far back I went.Still further investigation shows that this patch caused bug #16655 [1].
It should *not* have been designed to unconditionally clear the
table's "interesting" flag, as there may have been other reasons
why that was set. The right way to think about it is "if we are
going to dump the table's data, then the table certainly needs its
interesting flag set, so that we'll collect the per-attribute info.
Otherwise leave well enough alone".
Yes, I see the issue. Mea culpa :-(
The patches I proposed in the other thread seem like they really ought
to go all the way back for safety's sake. However, I do not observe
any crash on the test case in v11, and I'm kind of wondering why not.
Did you identify exactly where this was "introduced in release 12"?
It looks like you've since discovered the cause here. Do you need me to
dig more?
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services