create partition table caused server crashed with self-referencing foreign key
Hi,
Getting a server crash while creating partition table which have
self-referencing foreign key
postgres=# CREATE TABLE part1 (c1 int PRIMARY KEY, c2 int REFERENCES part1)
PARTITION BY LIST (c1);
CREATE TABLE
postgres=# CREATE TABLE part1_p1 PARTITION OF part1 FOR VALUES IN (1);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
--stack-trace
[edb@localhost bin]$ gdb -q -c data/core.16883 postgres
Core was generated by `postgres: edb postgres [local] CREATE TABLE
'.
Program terminated with signal 6, Aborted.
#0 0x00000039212324f5 in raise (sig=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
Missing separate debuginfos, use: debuginfo-install
keyutils-libs-1.4-5.el6.x86_64 krb5-libs-1.10.3-65.el6.x86_64
libcom_err-1.41.12-24.el6.x86_64 libgcc-4.4.7-23.el6.x86_64
libselinux-2.0.94-7.el6.x86_64 openssl-1.0.1e-58.el6_10.x86_64
zlib-1.2.3-29.el6.x86_64
(gdb) bt
#0 0x00000039212324f5 in raise (sig=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x0000003921233cd5 in abort () at abort.c:92
#2 0x0000000000acd16a in ExceptionalCondition (conditionName=0xc32310
"numfks == attmap->maplen", errorType=0xc2ea23 "FailedAssertion",
fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at assert.c:67
#3 0x00000000006d1b6c in CloneFkReferenced (parentRel=0x7f3c80be2400,
partitionRel=0x7f3c80be2a50) at tablecmds.c:9046
#4 0x00000000006d189b in CloneForeignKeyConstraints (wqueue=0x0,
parentRel=0x7f3c80be2400, partitionRel=0x7f3c80be2a50) at tablecmds.c:8939
#5 0x00000000006c09a8 in DefineRelation (stmt=0x2ff25b8, relkind=114 'r',
ownerId=10, typaddress=0x0, queryString=0x2f19810 "CREATE TABLE part1_p1
PARTITION OF part1 FOR VALUES IN (1);")
at tablecmds.c:1151
#6 0x0000000000953021 in ProcessUtilitySlow (pstate=0x2ff24a0,
pstmt=0x2f1a588, queryString=0x2f19810 "CREATE TABLE part1_p1 PARTITION OF
part1 FOR VALUES IN (1);", context=PROCESS_UTILITY_TOPLEVEL,
params=0x0, queryEnv=0x0, dest=0x2f1a868, qc=0x7ffffc1faa10) at
utility.c:1154
#7 0x0000000000952dfe in standard_ProcessUtility (pstmt=0x2f1a588,
queryString=0x2f19810 "CREATE TABLE part1_p1 PARTITION OF part1 FOR VALUES
IN (1);", context=PROCESS_UTILITY_TOPLEVEL, params=0x0,
queryEnv=0x0, dest=0x2f1a868, qc=0x7ffffc1faa10) at utility.c:1067
#8 0x0000000000951d18 in ProcessUtility (pstmt=0x2f1a588,
queryString=0x2f19810 "CREATE TABLE part1_p1 PARTITION OF part1 FOR VALUES
IN (1);", context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0,
dest=0x2f1a868, qc=0x7ffffc1faa10) at utility.c:522
#9 0x0000000000950b48 in PortalRunUtility (portal=0x2f808c0,
pstmt=0x2f1a588, isTopLevel=true, setHoldSnapshot=false, dest=0x2f1a868,
qc=0x7ffffc1faa10) at pquery.c:1157
#10 0x0000000000950d6e in PortalRunMulti (portal=0x2f808c0,
isTopLevel=true, setHoldSnapshot=false, dest=0x2f1a868, altdest=0x2f1a868,
qc=0x7ffffc1faa10) at pquery.c:1303
#11 0x000000000095023a in PortalRun (portal=0x2f808c0,
count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x2f1a868,
altdest=0x2f1a868, qc=0x7ffffc1faa10) at pquery.c:779
#12 0x000000000094a2a3 in exec_simple_query (query_string=0x2f19810 "CREATE
TABLE part1_p1 PARTITION OF part1 FOR VALUES IN (1);") at postgres.c:1239
#13 0x000000000094e38e in PostgresMain (argc=1, argv=0x2f44998,
dbname=0x2f448b0 "postgres", username=0x2f44890 "edb") at postgres.c:4315
#14 0x000000000089ba5d in BackendRun (port=0x2f3c7f0) at postmaster.c:4510
#15 0x000000000089b24c in BackendStartup (port=0x2f3c7f0) at
postmaster.c:4202
#16 0x00000000008975be in ServerLoop () at postmaster.c:1727
#17 0x0000000000896f07 in PostmasterMain (argc=3, argv=0x2f14240) at
postmaster.c:1400
#18 0x00000000007999cc in main (argc=3, argv=0x2f14240) at main.c:210
Thanks & Regards,
Rajkumar Raghuwanshi
On Wed, Apr 22, 2020 at 1:21 PM Rajkumar Raghuwanshi <
rajkumar.raghuwanshi@enterprisedb.com> wrote:
Hi,
Getting a server crash while creating partition table which have
self-referencing foreign keypostgres=# CREATE TABLE part1 (c1 int PRIMARY KEY, c2 int REFERENCES
part1) PARTITION BY LIST (c1);
CREATE TABLE
postgres=# CREATE TABLE part1_p1 PARTITION OF part1 FOR VALUES IN (1);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.--stack-trace
[edb@localhost bin]$ gdb -q -c data/core.16883 postgres
Core was generated by `postgres: edb postgres [local] CREATE TABLE
'.
Program terminated with signal 6, Aborted.
#0 0x00000039212324f5 in raise (sig=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
Missing separate debuginfos, use: debuginfo-install
keyutils-libs-1.4-5.el6.x86_64 krb5-libs-1.10.3-65.el6.x86_64
libcom_err-1.41.12-24.el6.x86_64 libgcc-4.4.7-23.el6.x86_64
libselinux-2.0.94-7.el6.x86_64 openssl-1.0.1e-58.el6_10.x86_64
zlib-1.2.3-29.el6.x86_64
(gdb) bt
#0 0x00000039212324f5 in raise (sig=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x0000003921233cd5 in abort () at abort.c:92
#2 0x0000000000acd16a in ExceptionalCondition (conditionName=0xc32310
"numfks == attmap->maplen", errorType=0xc2ea23 "FailedAssertion",
fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at assert.c:67
Looks like this assertion is incorrect, I guess it should have check
numfks <= attmap->maplen instead.
Regards,
Amul
On Wed, 22 Apr 2020 at 20:11, amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 1:21 PM Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com> wrote:
#2 0x0000000000acd16a in ExceptionalCondition (conditionName=0xc32310 "numfks == attmap->maplen", errorType=0xc2ea23 "FailedAssertion", fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at assert.c:67
Looks like this assertion is incorrect, I guess it should have check
numfks <= attmap->maplen instead.
Even that seems like a very strange thing to Assert. Basically it's
saying, make sure the number of columns in the foreign key constraint
is less than or equal to the number of attributes in parentRel.
It's true we do disallow duplicate column names in the foreign key
constraint (at least since 9da867537), but why do we want an Assert to
say that? I don't see anything about that code that would break if we
did happen to allow duplicate columns in the foreign key. I'd say the
Assert should just be removed completely.
David
On Wed, Apr 22, 2020 at 2:27 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Wed, 22 Apr 2020 at 20:11, amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 1:21 PM Rajkumar Raghuwanshi <
rajkumar.raghuwanshi@enterprisedb.com> wrote:
#2 0x0000000000acd16a in ExceptionalCondition (conditionName=0xc32310
"numfks == attmap->maplen", errorType=0xc2ea23 "FailedAssertion",
fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at assert.c:67Looks like this assertion is incorrect, I guess it should have check
numfks <= attmap->maplen instead.Even that seems like a very strange thing to Assert. Basically it's
saying, make sure the number of columns in the foreign key constraint
is less than or equal to the number of attributes in parentRel.It's true we do disallow duplicate column names in the foreign key
constraint (at least since 9da867537), but why do we want an Assert to
say that? I don't see anything about that code that would break if we
did happen to allow duplicate columns in the foreign key. I'd say the
Assert should just be removed completely.
Understood and agree with you.
Attached patch removes this assertion and does a slight tweak to regression
test
to generate case where numfks != attmap->maplen, IMO, we should have this
even if there is nothing that checks it. Thoughts?
Regards,
Amul
Attachments:
0001-Remove-unwanted-assert-check-and-a-slight-tweak-to-t.patchapplication/octet-stream; name=0001-Remove-unwanted-assert-check-and-a-slight-tweak-to-t.patchDownload
From 70d15088448f01a4c8ae6b4d0fe856fab94229a5 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Wed, 22 Apr 2020 05:17:49 -0400
Subject: [PATCH] Remove unwanted assert check and a slight tweak to test.
---
src/backend/commands/tablecmds.c | 1 -
src/test/regress/sql/foreign_key.sql | 8 ++++----
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 037d457c3d4..5dddc20964f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9042,7 +9042,6 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
conpfeqop,
conppeqop,
conffeqop);
- Assert(numfks == attmap->maplen);
for (int i = 0; i < numfks; i++)
mapped_confkey[i] = attmap->attnums[confkey[i] - 1];
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index c5c9011afcb..922d5d2c229 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1214,7 +1214,7 @@ DROP TABLE fk_notpartitioned_pk, fk_partitioned_fk;
-- Altering a type referenced by a foreign key needs to drop/recreate the FK.
-- Ensure that works.
-CREATE TABLE fk_notpartitioned_pk (a INT, PRIMARY KEY(a), CHECK (a > 0));
+CREATE TABLE fk_notpartitioned_pk (a INT, PRIMARY KEY(a), CHECK (a > 0), c int);
CREATE TABLE fk_partitioned_fk (a INT REFERENCES fk_notpartitioned_pk(a) PRIMARY KEY) PARTITION BY RANGE(a);
CREATE TABLE fk_partitioned_fk_1 PARTITION OF fk_partitioned_fk FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
INSERT INTO fk_notpartitioned_pk VALUES (1);
@@ -1483,9 +1483,9 @@ drop schema fkpart0, fkpart1, fkpart2, fkpart3 cascade;
CREATE SCHEMA fkpart3;
SET search_path TO fkpart3;
-CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a);
+CREATE TABLE pk (a int PRIMARY KEY, c int) PARTITION BY RANGE (a);
CREATE TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (1000);
-CREATE TABLE pk2 (b int, a int);
+CREATE TABLE pk2 (b int, a int, c int);
ALTER TABLE pk2 DROP COLUMN b;
ALTER TABLE pk2 ALTER a SET NOT NULL;
ALTER TABLE pk ATTACH PARTITION pk2 FOR VALUES FROM (1000) TO (2000);
@@ -1493,7 +1493,7 @@ ALTER TABLE pk ATTACH PARTITION pk2 FOR VALUES FROM (1000) TO (2000);
CREATE TABLE fk (a int) PARTITION BY RANGE (a);
CREATE TABLE fk1 PARTITION OF fk FOR VALUES FROM (0) TO (750);
ALTER TABLE fk ADD FOREIGN KEY (a) REFERENCES pk;
-CREATE TABLE fk2 (b int, a int) ;
+CREATE TABLE fk2 (b int, a int, c int);
ALTER TABLE fk2 DROP COLUMN b;
ALTER TABLE fk ATTACH PARTITION fk2 FOR VALUES FROM (750) TO (3500);
--
2.18.0
On Wed, Apr 22, 2020 at 2:59 PM amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 2:27 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Wed, 22 Apr 2020 at 20:11, amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 1:21 PM Rajkumar Raghuwanshi <
rajkumar.raghuwanshi@enterprisedb.com> wrote:
#2 0x0000000000acd16a in ExceptionalCondition (conditionName=0xc32310
"numfks == attmap->maplen", errorType=0xc2ea23 "FailedAssertion",
fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at assert.c:67Looks like this assertion is incorrect, I guess it should have check
numfks <= attmap->maplen instead.Even that seems like a very strange thing to Assert. Basically it's
saying, make sure the number of columns in the foreign key constraint
is less than or equal to the number of attributes in parentRel.It's true we do disallow duplicate column names in the foreign key
constraint (at least since 9da867537), but why do we want an Assert to
say that? I don't see anything about that code that would break if we
did happen to allow duplicate columns in the foreign key. I'd say the
Assert should just be removed completely.Understood and agree with you.
Attached patch removes this assertion and does a slight tweak to
regression test
to generate case where numfks != attmap->maplen, IMO, we should have this
even if there is nothing that checks it. Thoughts?
Kindly ignore the previously attached patch, correct patch attached here.
Regards,
Amul
Attachments:
v2-0001-Remove-unwanted-assert-check-and-a-slight-tweak-t.patchapplication/octet-stream; name=v2-0001-Remove-unwanted-assert-check-and-a-slight-tweak-t.patchDownload
From 28fa04ddcb95906e4cab659f2c835e9099219db3 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Wed, 22 Apr 2020 05:42:12 -0400
Subject: [PATCH v2] Remove unwanted assert check and a slight tweak to test.
---
src/backend/commands/tablecmds.c | 1 -
src/test/regress/expected/foreign_key.out | 8 ++++----
src/test/regress/sql/foreign_key.sql | 8 ++++----
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 037d457c3d4..5dddc20964f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9042,7 +9042,6 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
conpfeqop,
conppeqop,
conffeqop);
- Assert(numfks == attmap->maplen);
for (int i = 0; i < numfks; i++)
mapped_confkey[i] = attmap->attnums[confkey[i] - 1];
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 07bd5b6434f..039777500d6 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2075,9 +2075,9 @@ drop cascades to table fkpart0.fk_part
-- attached) after creating the foreign key.
CREATE SCHEMA fkpart3;
SET search_path TO fkpart3;
-CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a);
+CREATE TABLE pk (a int PRIMARY KEY, d int) PARTITION BY RANGE (a);
CREATE TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (1000);
-CREATE TABLE pk2 (b int, a int);
+CREATE TABLE pk2 (b int, a int, d int);
ALTER TABLE pk2 DROP COLUMN b;
ALTER TABLE pk2 ALTER a SET NOT NULL;
ALTER TABLE pk ATTACH PARTITION pk2 FOR VALUES FROM (1000) TO (2000);
@@ -2090,7 +2090,7 @@ ALTER TABLE fk ATTACH PARTITION fk2 FOR VALUES FROM (750) TO (3500);
CREATE TABLE pk3 PARTITION OF pk FOR VALUES FROM (2000) TO (3000);
CREATE TABLE pk4 (LIKE pk);
ALTER TABLE pk ATTACH PARTITION pk4 FOR VALUES FROM (3000) TO (4000);
-CREATE TABLE pk5 (c int, b int, a int NOT NULL) PARTITION BY RANGE (a);
+CREATE TABLE pk5 (c int, b int, a int NOT NULL, d int) PARTITION BY RANGE (a);
ALTER TABLE pk5 DROP COLUMN b, DROP COLUMN c;
CREATE TABLE pk51 PARTITION OF pk5 FOR VALUES FROM (4000) TO (4500);
CREATE TABLE pk52 PARTITION OF pk5 FOR VALUES FROM (4500) TO (5000);
@@ -2116,7 +2116,7 @@ INSERT into fk VALUES (4500);
ERROR: insert or update on table "fk3" violates foreign key constraint "fk_a_fkey"
DETAIL: Key (a)=(4500) is not present in table "pk".
-- insert into the referenced table, now they should work
-INSERT into pk VALUES (1), (1000), (2000), (3000), (4000), (4500);
+INSERT into pk(a) VALUES (1), (1000), (2000), (3000), (4000), (4500);
INSERT into fk VALUES (1), (1000), (2000), (3000), (4000), (4500);
-- should fail: referencing value present
DELETE FROM pk WHERE a = 1;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index c5c9011afcb..c4949e31db9 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1483,9 +1483,9 @@ drop schema fkpart0, fkpart1, fkpart2, fkpart3 cascade;
CREATE SCHEMA fkpart3;
SET search_path TO fkpart3;
-CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a);
+CREATE TABLE pk (a int PRIMARY KEY, d int) PARTITION BY RANGE (a);
CREATE TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (1000);
-CREATE TABLE pk2 (b int, a int);
+CREATE TABLE pk2 (b int, a int, d int);
ALTER TABLE pk2 DROP COLUMN b;
ALTER TABLE pk2 ALTER a SET NOT NULL;
ALTER TABLE pk ATTACH PARTITION pk2 FOR VALUES FROM (1000) TO (2000);
@@ -1501,7 +1501,7 @@ CREATE TABLE pk3 PARTITION OF pk FOR VALUES FROM (2000) TO (3000);
CREATE TABLE pk4 (LIKE pk);
ALTER TABLE pk ATTACH PARTITION pk4 FOR VALUES FROM (3000) TO (4000);
-CREATE TABLE pk5 (c int, b int, a int NOT NULL) PARTITION BY RANGE (a);
+CREATE TABLE pk5 (c int, b int, a int NOT NULL, d int) PARTITION BY RANGE (a);
ALTER TABLE pk5 DROP COLUMN b, DROP COLUMN c;
CREATE TABLE pk51 PARTITION OF pk5 FOR VALUES FROM (4000) TO (4500);
CREATE TABLE pk52 PARTITION OF pk5 FOR VALUES FROM (4500) TO (5000);
@@ -1517,7 +1517,7 @@ INSERT into fk VALUES (3000);
INSERT into fk VALUES (4000);
INSERT into fk VALUES (4500);
-- insert into the referenced table, now they should work
-INSERT into pk VALUES (1), (1000), (2000), (3000), (4000), (4500);
+INSERT into pk(a) VALUES (1), (1000), (2000), (3000), (4000), (4500);
INSERT into fk VALUES (1), (1000), (2000), (3000), (4000), (4500);
-- should fail: referencing value present
--
2.18.0
On Wed, 22 Apr 2020 at 21:30, amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 2:27 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Wed, 22 Apr 2020 at 20:11, amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 1:21 PM Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com> wrote:
#2 0x0000000000acd16a in ExceptionalCondition (conditionName=0xc32310 "numfks == attmap->maplen", errorType=0xc2ea23 "FailedAssertion", fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at assert.c:67
Looks like this assertion is incorrect, I guess it should have check
numfks <= attmap->maplen instead.Even that seems like a very strange thing to Assert. Basically it's
saying, make sure the number of columns in the foreign key constraint
is less than or equal to the number of attributes in parentRel.It's true we do disallow duplicate column names in the foreign key
constraint (at least since 9da867537), but why do we want an Assert to
say that? I don't see anything about that code that would break if we
did happen to allow duplicate columns in the foreign key. I'd say the
Assert should just be removed completely.Understood and agree with you.
I pushed a patch to remove the Assert. I didn't really feel a need to
make any adjustments to the regression tests for this. The Assert was
clearly out of place, it's hard to imagine that this could ever get
broken again.
David
On Wed, Apr 22, 2020 at 2:45 PM amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 2:59 PM amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 2:27 PM David Rowley <dgrowleyml@gmail.com>
wrote:On Wed, 22 Apr 2020 at 20:11, amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 1:21 PM Rajkumar Raghuwanshi <
rajkumar.raghuwanshi@enterprisedb.com> wrote:
#2 0x0000000000acd16a in ExceptionalCondition
(conditionName=0xc32310 "numfks == attmap->maplen", errorType=0xc2ea23
"FailedAssertion", fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at
assert.c:67Looks like this assertion is incorrect, I guess it should have check
numfks <= attmap->maplen instead.Even that seems like a very strange thing to Assert. Basically it's
saying, make sure the number of columns in the foreign key constraint
is less than or equal to the number of attributes in parentRel.It's true we do disallow duplicate column names in the foreign key
constraint (at least since 9da867537), but why do we want an Assert to
say that? I don't see anything about that code that would break if we
did happen to allow duplicate columns in the foreign key. I'd say the
Assert should just be removed completely.Understood and agree with you.
Attached patch removes this assertion and does a slight tweak to
regression test
to generate case where numfks != attmap->maplen, IMO, we should have this
even if there is nothing that checks it. Thoughts?Kindly ignore the previously attached patch, correct patch attached here.
I did a quick test of the fix, the assertion failure is fixed and
regression is not reporting any failures.
Regards,
Amul
--
Highgo Software (Canada/China/Pakistan)
URL : http://www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
EMAIL: mailto: ahsan.hadi@highgo.ca
Thanks all for quick fix and push.
Thanks & Regards,
Rajkumar Raghuwanshi
On Wed, Apr 22, 2020 at 4:14 PM Ahsan Hadi <ahsan.hadi@gmail.com> wrote:
Show quoted text
On Wed, Apr 22, 2020 at 2:45 PM amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 2:59 PM amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 2:27 PM David Rowley <dgrowleyml@gmail.com>
wrote:On Wed, 22 Apr 2020 at 20:11, amul sul <sulamul@gmail.com> wrote:
On Wed, Apr 22, 2020 at 1:21 PM Rajkumar Raghuwanshi <
rajkumar.raghuwanshi@enterprisedb.com> wrote:
#2 0x0000000000acd16a in ExceptionalCondition
(conditionName=0xc32310 "numfks == attmap->maplen", errorType=0xc2ea23
"FailedAssertion", fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at
assert.c:67Looks like this assertion is incorrect, I guess it should have check
numfks <= attmap->maplen instead.Even that seems like a very strange thing to Assert. Basically it's
saying, make sure the number of columns in the foreign key constraint
is less than or equal to the number of attributes in parentRel.It's true we do disallow duplicate column names in the foreign key
constraint (at least since 9da867537), but why do we want an Assert to
say that? I don't see anything about that code that would break if we
did happen to allow duplicate columns in the foreign key. I'd say the
Assert should just be removed completely.Understood and agree with you.
Attached patch removes this assertion and does a slight tweak to
regression test
to generate case where numfks != attmap->maplen, IMO, we should have
this
even if there is nothing that checks it. Thoughts?Kindly ignore the previously attached patch, correct patch attached here.
I did a quick test of the fix, the assertion failure is fixed and
regression is not reporting any failures.Regards,
Amul--
Highgo Software (Canada/China/Pakistan)
URL : http://www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
EMAIL: mailto: ahsan.hadi@highgo.ca
On Wed, Apr 22, 2020 at 10:21:21PM +1200, David Rowley wrote:
I pushed a patch to remove the Assert. I didn't really feel a need to
make any adjustments to the regression tests for this. The Assert was
clearly out of place, it's hard to imagine that this could ever get
broken again.
Still, it seems to me that there could be a point in having a test for
partitioned tables with FKs referencing themselves. We have such
tests for plain tables for example.
--
Michael
On Wed, 22 Apr 2020 at 23:50, Michael Paquier <michael@paquier.xyz> wrote:
On Wed, Apr 22, 2020 at 10:21:21PM +1200, David Rowley wrote:
I pushed a patch to remove the Assert. I didn't really feel a need to
make any adjustments to the regression tests for this. The Assert was
clearly out of place, it's hard to imagine that this could ever get
broken again.Still, it seems to me that there could be a point in having a test for
partitioned tables with FKs referencing themselves. We have such
tests for plain tables for example.
The reason I didn't take the additional tests that were proposed by
Amul was that I didn't think that they really added any additional
coverage to the code that remained. They would only have served to
ensure that nobody went and added the same Assert back again. Since
the Assert was clearly out of place, I didn't think it was worthy of
burdening our build farm with the additional overhead of the modified
test from now until the end of time.
I'd put it akin to fixing a spelling mistake in an error message and
adding a special test specifically to ensure the spelling is correct.
Would we add a test for that? Likely not. Would someone reintroduce
the spelling mistake again? Likely not.
I think discussions for any tests beyond the scope of this fix are
probably for another thread. I'm happy to join any discussions about
it there.
David