Обсуждение: create partition table caused server crashed with self-referencingforeign 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

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.

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. 

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?

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: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. 

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