Horton

Thread

Commits

  1. Fix typos in simplehash.h

  2. Verify that expected slot types match returned slot types.

  1. BUG #18252: Assert in CheckOpSlotCompatibility() fails when recursive union filters tuples in non-recursive term

    The Post Office <noreply@postgresql.org> — 2023-12-18T16:00:02Z

    The following bug has been logged on the website:

Bug reference:      18252
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 16.1
Operating system:   Ubuntu 22.04
Description:        

The following query:
CREATE TABLE t(i int);
INSERT INTO t VALUES (1), (1);

WITH RECURSIVE rt(i) AS (
    SELECT * FROM t
    UNION
    SELECT * FROM rt
)
SELECT * FROM rt;

triggers an assertion failure with the following call stack:
Core was generated by `postgres: law regression [local] SELECT              
                        '.
Program terminated with signal SIGABRT, Aborted.

warning: Section `.reg-xstate/1408945' in core file too small.
#0  __pthread_kill_implementation (no_tid=0, signo=6,
threadid=140277092927296) at ./nptl/pthread_kill.c:44
44      ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
#0  __pthread_kill_implementation (no_tid=0, signo=6,
threadid=140277092927296) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=140277092927296) at
./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=140277092927296, signo=signo@entry=6) at
./nptl/pthread_kill.c:89
#3  0x00007f94cdaf0476 in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
#4  0x00007f94cdad67f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x000056431a46890d in ExceptionalCondition (conditionName=0x56431a5fe928
"op->d.fetch.kind == slot->tts_ops", fileName=0x56431a5fe64a
"execExprInterp.c", lineNumber=2005) at assert.c:66
#6  0x0000564319fbeca1 in CheckOpSlotCompatibility (op=0x56431c3cee30,
slot=0x56431c3ce588) at execExprInterp.c:2005
#7  0x0000564319fbb9d5 in ExecInterpExpr (state=0x56431c3ceda0,
econtext=0x56431c3cf330, isnull=0x7ffd627dddff) at execExprInterp.c:534
#8  0x0000564319fbe8d9 in ExecInterpExprStillValid (state=0x56431c3ceda0,
econtext=0x56431c3cf330, isNull=0x7ffd627dddff) at execExprInterp.c:1870
#9  0x0000564319fc487f in ExecEvalExprSwitchContext (state=0x56431c3ceda0,
econtext=0x56431c3cf330, isNull=0x7ffd627dddff) at
../../../src/include/executor/executor.h:355
#10 0x0000564319fc4909 in ExecQual (state=0x56431c3ceda0,
econtext=0x56431c3cf330) at ../../../src/include/executor/executor.h:424
#11 0x0000564319fc4978 in ExecQualAndReset (state=0x56431c3ceda0,
econtext=0x56431c3cf330) at ../../../src/include/executor/executor.h:441
#12 0x0000564319fc6957 in TupleHashTableMatch (tb=0x56431c3cebb0,
tuple1=0x56431c4032e8, tuple2=0x0) at execGrouping.c:559
#13 0x0000564319fc5240 in tuplehash_insert_hash_internal (tb=0x56431c3cebb0,
key=0x0, hash=2072154383, found=0x7ffd627ddf77) at
../../../src/include/lib/simplehash.h:653
#14 0x0000564319fc559b in tuplehash_insert_hash (tb=0x56431c3cebb0, key=0x0,
hash=2072154383, found=0x7ffd627ddf77) at
../../../src/include/lib/simplehash.h:774
#15 0x0000564319fc67fa in LookupTupleHashEntry_internal
(hashtable=0x56431c3ceb20, slot=0x56431c3ce588, isnew=0x7ffd627de00f,
hash=2072154383) at execGrouping.c:507
#16 0x0000564319fc642e in LookupTupleHashEntry (hashtable=0x56431c3ceb20,
slot=0x56431c3ce588, isnew=0x7ffd627de00f, hash=0x0) at execGrouping.c:322
#17 0x000056431a01bc1e in ExecRecursiveUnion (pstate=0x56431c3cdc38) at
nodeRecursiveunion.c:97
#18 0x0000564319ff3c92 in ExecProcNode (node=0x56431c3cdc38) at
../../../src/include/executor/executor.h:273
#19 0x0000564319ff3dd9 in CteScanNext (node=0x56431c3cf410) at
nodeCtescan.c:103
#20 0x0000564319fda4d3 in ExecScanFetch (node=0x56431c3cf410,
accessMtd=0x564319ff3c94 <CteScanNext>, recheckMtd=0x564319ff3e5d
<CteScanRecheck>) at execScan.c:132
#21 0x0000564319fda54c in ExecScan (node=0x56431c3cf410,
accessMtd=0x564319ff3c94 <CteScanNext>, recheckMtd=0x564319ff3e5d
<CteScanRecheck>) at execScan.c:181
#22 0x0000564319ff3eb6 in ExecCteScan (pstate=0x56431c3cf410) at
nodeCtescan.c:164
#23 0x0000564319fc8f3d in ExecProcNode (node=0x56431c3cf410) at
../../../src/include/executor/executor.h:273
#24 0x0000564319fcbef5 in ExecutePlan (estate=0x56431c3cd968,
planstate=0x56431c3cf410, use_parallel_mode=false, operation=CMD_SELECT,
sendTuples=true, numberTuples=0, direction=ForwardScanDirection,
dest=0x56431c3dfd40, 
    execute_once=true) at execMain.c:1670
#25 0x0000564319fc9625 in standard_ExecutorRun (queryDesc=0x56431c320c18,
direction=ForwardScanDirection, count=0, execute_once=true) at
execMain.c:365
#26 0x0000564319fc942e in ExecutorRun (queryDesc=0x56431c320c18,
direction=ForwardScanDirection, count=0, execute_once=true) at
execMain.c:309
#27 0x000056431a2863ec in PortalRunSelect (portal=0x56431c371cd8,
forward=true, count=0, dest=0x56431c3dfd40) at pquery.c:924
#28 0x000056431a286014 in PortalRun (portal=0x56431c371cd8,
count=9223372036854775807, isTopLevel=true, run_once=true,
dest=0x56431c3dfd40, altdest=0x56431c3dfd40, qc=0x7ffd627de520) at
pquery.c:768
#29 0x000056431a27edd4 in exec_simple_query (query_string=0x56431c2f67a8
"WITH RECURSIVE rt(i) AS (\n    SELECT * FROM t\n    UNION\n    SELECT *
FROM rt\n)\nSELECT * FROM rt;") at postgres.c:1274
#30 0x000056431a283e42 in PostgresMain (dbname=0x56431c32fc38 "regression",
username=0x56431c2f27d8 "law") at postgres.c:4637
#31 0x000056431a1a4c5e in BackendRun (port=0x56431c3230a0) at
postmaster.c:4464
#32 0x000056431a1a44ea in BackendStartup (port=0x56431c3230a0) at
postmaster.c:4192
#33 0x000056431a1a082f in ServerLoop () at postmaster.c:1782
#34 0x000056431a1a00d9 in PostmasterMain (argc=3, argv=0x56431c2f0710) at
postmaster.c:1466
#35 0x000056431a054518 in main (argc=3, argv=0x56431c2f0710) at main.c:198

(gdb) frame 6
#6  0x0000564319fbeca1 in CheckOpSlotCompatibility (op=0x56431c3cee30,
slot=0x56431c3ce588)
    at execExprInterp.c:2005
2005            Assert(op->d.fetch.kind == slot->tts_ops);
(gdb) p op->d.fetch.kind
$1 = (const TupleTableSlotOps *) 0x56431a7ebcc0 <TTSOpsMinimalTuple>
(gdb) p slot->tts_ops
$2 = (const TupleTableSlotOps * const) 0x56431a7ebd20
<TTSOpsBufferHeapTuple>

With "INSERT INTO t VALUES (1), (2)" the assert is not reached during
processing of the non-recursive term.

Reproduced on REL_12_STABLE .. master.
This check failure can be observed since the introduction of
CheckOpSlotCompatibility (15d8f8312) + f92cd7392.

  2. Re: BUG #18252: Assert in CheckOpSlotCompatibility() fails when recursive union filters tuples in non-recursive term

    Richard Guo <guofenglinux@gmail.com> — 2023-12-19T11:28:35Z

    On Tue, Dec 19, 2023 at 12:30 AM PG Bug reporting form <
noreply@postgresql.org> wrote:

> CREATE TABLE t(i int);
> INSERT INTO t VALUES (1), (1);
>
> WITH RECURSIVE rt(i) AS (
>     SELECT * FROM t
>     UNION
>     SELECT * FROM rt
> )
> SELECT * FROM rt;


Nice catch.  The TupleHashTable's tableslot is created as type
TTSOpsMinimalTuple.  The RecursiveUnion's non-recursive term generates
tuples of type TTSOpsBufferHeapTuple.  If the non-recursive term
produces duplicate tuples, we'd find non-empty bucket when inserting the
same key into the hashtable.  In this case we need to check to see if
the two tuples match: one is of the hashtable's tableslot
(TTSOpsMinimalTuple), and the other comes from the non-recursive term
(TTSOpsBufferHeapTuple).  And the incompatible slot type would fail
CheckOpSlotCompatibility.

I have no idea about how to fix it.

BTW, while reading the codes, I noticed two typos in simplehash.h, one
is in the comment of SH_INSERT, the other is in the comment of
SH_INSERT_HASH.  Attached is a trivial patch for that.

Thanks
Richard

  3. Re: BUG #18252: Assert in CheckOpSlotCompatibility() fails when recursive union filters tuples in non-recursive term

    Peter Eisentraut <peter@eisentraut.org> — 2023-12-26T14:42:30Z

    On 19.12.23 12:28, Richard Guo wrote:
> BTW, while reading the codes, I noticed two typos in simplehash.h, one
> is in the comment of SH_INSERT, the other is in the comment of
> SH_INSERT_HASH.  Attached is a trivial patch for that.

  /*
- * Insert the key key into the hash-table, set *found to true if the key
- * already exists, false otherwise. Returns the hash-table entry in either
- * case.
+ * Insert the key into the hash-table, set *found to true if the key 
already
+ * exists, false otherwise. Returns the hash-table entry in either case.
   */
  SH_SCOPE       SH_ELEMENT_TYPE *
  SH_INSERT(SH_TYPE * tb, SH_KEY_TYPE key, bool *found)


I suppose the original intention might have been "Insert the key passed 
in the parameter named key ..." or "Insert the key `key` ..." or 
something like that.  But even if so, that's clearly confusing, so I am 
in favor of your patch, just pointing this out in case someone wants to 
protest.

  4. Re: BUG #18252: Assert in CheckOpSlotCompatibility() fails when recursive union filters tuples in non-recursive term

    Peter Eisentraut <peter@eisentraut.org> — 2024-01-02T09:22:34Z

    On 26.12.23 15:42, Peter Eisentraut wrote:
> On 19.12.23 12:28, Richard Guo wrote:
>> BTW, while reading the codes, I noticed two typos in simplehash.h, one
>> is in the comment of SH_INSERT, the other is in the comment of
>> SH_INSERT_HASH.  Attached is a trivial patch for that.
> 
>   /*
> - * Insert the key key into the hash-table, set *found to true if the key
> - * already exists, false otherwise. Returns the hash-table entry in either
> - * case.
> + * Insert the key into the hash-table, set *found to true if the key 
> already
> + * exists, false otherwise. Returns the hash-table entry in either case.
>    */
>   SH_SCOPE       SH_ELEMENT_TYPE *
>   SH_INSERT(SH_TYPE * tb, SH_KEY_TYPE key, bool *found)
> 
> 
> I suppose the original intention might have been "Insert the key passed 
> in the parameter named key ..." or "Insert the key `key` ..." or 
> something like that.  But even if so, that's clearly confusing, so I am 
> in favor of your patch, just pointing this out in case someone wants to 
> protest.

hearing none, committed

  5. Re: BUG #18252: Assert in CheckOpSlotCompatibility() fails when recursive union filters tuples in non-recursive term

    Richard Guo <guofenglinux@gmail.com> — 2024-01-10T07:44:13Z

    On Tue, Jan 2, 2024 at 5:22 PM Peter Eisentraut <peter@eisentraut.org>
wrote:

> On 26.12.23 15:42, Peter Eisentraut wrote:
> > On 19.12.23 12:28, Richard Guo wrote:
> >> BTW, while reading the codes, I noticed two typos in simplehash.h, one
> >> is in the comment of SH_INSERT, the other is in the comment of
> >> SH_INSERT_HASH.  Attached is a trivial patch for that.
> >
> >   /*
> > - * Insert the key key into the hash-table, set *found to true if the key
> > - * already exists, false otherwise. Returns the hash-table entry in
> either
> > - * case.
> > + * Insert the key into the hash-table, set *found to true if the key
> > already
> > + * exists, false otherwise. Returns the hash-table entry in either case.
> >    */
> >   SH_SCOPE       SH_ELEMENT_TYPE *
> >   SH_INSERT(SH_TYPE * tb, SH_KEY_TYPE key, bool *found)
> >
> >
> > I suppose the original intention might have been "Insert the key passed
> > in the parameter named key ..." or "Insert the key `key` ..." or
> > something like that.  But even if so, that's clearly confusing, so I am
> > in favor of your patch, just pointing this out in case someone wants to
> > protest.
>
> hearing none, committed


Thanks for pushing!

Thanks
Richard

  6. Re: BUG #18252: Assert in CheckOpSlotCompatibility() fails when recursive union filters tuples in non-recursive term

    Richard Guo <guofenglinux@gmail.com> — 2024-01-10T07:46:12Z

    On Tue, Dec 19, 2023 at 7:28 PM Richard Guo <guofenglinux@gmail.com> wrote:

> On Tue, Dec 19, 2023 at 12:30 AM PG Bug reporting form <
> noreply@postgresql.org> wrote:
>
>> CREATE TABLE t(i int);
>> INSERT INTO t VALUES (1), (1);
>>
>> WITH RECURSIVE rt(i) AS (
>>     SELECT * FROM t
>>     UNION
>>     SELECT * FROM rt
>> )
>> SELECT * FROM rt;
>
>
> Nice catch.  The TupleHashTable's tableslot is created as type
> TTSOpsMinimalTuple.  The RecursiveUnion's non-recursive term generates
> tuples of type TTSOpsBufferHeapTuple.  If the non-recursive term
> produces duplicate tuples, we'd find non-empty bucket when inserting the
> same key into the hashtable.  In this case we need to check to see if
> the two tuples match: one is of the hashtable's tableslot
> (TTSOpsMinimalTuple), and the other comes from the non-recursive term
> (TTSOpsBufferHeapTuple).  And the incompatible slot type would fail
> CheckOpSlotCompatibility.
>

This Assert was added in 15d8f8312:

commit 15d8f83128e15de97de61430d0b9569f5ebecc26
Author: Andres Freund <andres@anarazel.de>
Date:   Thu Nov 15 22:00:30 2018 -0800

    Verify that expected slot types match returned slot types.

So loop in Andres to have a look.

Thanks
Richard