Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Further fixes in qual nullingrel adjustment for outer join commutation.

  2. Further tighten nullingrel marking rules in build_joinrel_tlist().

  3. remove_rel_from_query() must clean up PlaceHolderVar.phrels fields.

  4. Rethink nullingrel marking rules in build_joinrel_tlist().

  5. Make Vars be outer-join-aware.

  1. BUG #17781: Assert in setrefs.c

    PG Bug reporting form <noreply@postgresql.org> — 2023-02-07T13:16:08Z

    The following bug has been logged on the website:

Bug reference:      17781
Logged by:          Robins Tharakan
Email address:      tharakan@gmail.com
PostgreSQL version: 15.1
Operating system:   Ubuntu 20.04
Description:        

This assert() is easily reproducible as of aa69541046@master, although I can
trace the issue back to last week's commit 2489d76c49.

TRAP: failed Assert("nrm_match == NRM_SUBSET ?
bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match ==
NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels, phv->phnullingrels) :
bms_equal(subphv->phnullingrels, phv->phnullingrels)"), File: "setrefs.c",
Line: 2845, PID: 3217440

Backtrace / SQL / backtrace full (excerpt) below.


SQL
===
rollback;
begin;
create table t6();
create table n(a numeric);
create table t(a text);
create table l();
create table n2(n numeric);
create table f();
SELECT subq_3.c1 AS c8
FROM t6 AS sample_0
     LEFT JOIN (n AS ref_1
                RIGHT JOIN f AS sample_1 ON NULL
                LEFT JOIN (n2 AS sample_3
                           RIGHT JOIN t AS sample_4 ON NULL)
                   ON ref_1.a = sample_3.n
                LEFT JOIN ((SELECT NULL AS c1
                            FROM l AS ref_2) AS subq_3
                           INNER JOIN pg_catalog.pg_stat_gssapi AS ref_4 ON
NULL)
                   ON ref_4.principal !~~ sample_4.a)
        ON NULL;


Checking (aa69541046~49) - b448f1c8d8 - Crash
Checking (aa69541046~50) - 2489d76c49 - Crash
Checking (aa69541046~51) - ec7e053a98 - Success
Checking (aa69541046~52) - fe9e658f4d - Success



Backtrace
=========
Core was generated by `postgres: aa69541046@master@sqith: ubuntu postgres
127.0.'.
Program terminated with signal SIGABRT, Aborted.
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007f5d5b59f859 in __GI_abort () at abort.c:79
#2  0x000055dbe02e80d7 in ExceptionalCondition (
    conditionName=0x55dbe049fec0 "nrm_match == NRM_SUBSET ?
bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match ==
NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels, phv->phnullingrels) :
bms_equal(subphv->phnullingr"..., fileName=0x55dbe049f8ef "setrefs.c",
lineNumber=2845) at assert.c:66
#3  0x000055dbdffd7e28 in search_indexed_tlist_for_phv
(phv=0x55dbe14dc5c8,
    itlist=0x55dbe14e8a60, newvarno=-1, nrm_match=NRM_SUPERSET)
    at setrefs.c:2845
#4  0x000055dbdffd82df in fix_join_expr_mutator (node=0x55dbe14dc5c8,
    context=0x7ffdf5c7c660) at setrefs.c:3066
#5  0x000055dbdff14963 in expression_tree_mutator_impl
(node=0x55dbe14e8840,
    mutator=0x55dbdffd80b7 <fix_join_expr_mutator>,
context=0x7ffdf5c7c660)
    at nodeFuncs.c:3051
#6  0x000055dbdffd843b in fix_join_expr_mutator (node=0x55dbe14e8840,
    context=0x7ffdf5c7c660) at setrefs.c:3104
#7  0x000055dbdff14d6e in expression_tree_mutator_impl
(node=0x55dbe14e8890,
    mutator=0x55dbdffd80b7 <fix_join_expr_mutator>,
context=0x7ffdf5c7c660)
    at nodeFuncs.c:3137
#8  0x000055dbdffd843b in fix_join_expr_mutator (node=0x55dbe14e8890,
    context=0x7ffdf5c7c660) at setrefs.c:3104
#9  0x000055dbdffd80a1 in fix_join_expr (root=0x55dbe14d4f80,
    clauses=0x55dbe14e8890, outer_itlist=0x55dbe14e8a10,
    inner_itlist=0x55dbe14e8a60, acceptable_rel=0, rtoffset=0,
    nrm_match=NRM_SUPERSET, num_exec=2910) at setrefs.c:2999
#10 0x000055dbdffd7111 in set_join_references (root=0x55dbe14d4f80,
    join=0x55dbe14e87b0, rtoffset=0) at setrefs.c:2350
#11 0x000055dbdffd3ef9 in set_plan_refs (root=0x55dbe14d4f80,
    plan=0x55dbe14e87b0, rtoffset=0) at setrefs.c:870
#12 0x000055dbdffd2a1f in set_plan_references (root=0x55dbe14d4f80,
    plan=0x55dbe14e87b0) at setrefs.c:354
#13 0x000055dbdffc43cd in standard_planner (parse=0x55dbe14bd2c8,
    query_string=0x55dbe13951a8 "SELECT subq_3.c1 AS c8\nFROM t6 AS
sample_0\n     LEFT JOIN (n AS ref_1\n", ' ' <repeats 16 times>, "RIGHT JOIN
f AS sample_1 ON NULL\n", ' ' <repeats 16 times>, "LEFT JOIN (n2 AS
sample_3\n", ' ' <repeats 27 times>, "RIGHT JOIN t"..., cursorOptions=2048,
boundParams=0x0)
    at planner.c:504



Backtrace full excerpt
======================
#2  0x000055dbe02e80d7 in ExceptionalCondition (
    conditionName=0x55dbe049fec0 "nrm_match == NRM_SUBSET ?
bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match ==
NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels, phv->phnullingrels) :
bms_equal(subphv->phnullingr"..., fileName=0x55dbe049f8ef "setrefs.c",
lineNumber=2845) at assert.c:66
No locals.
#3  0x000055dbdffd7e28 in search_indexed_tlist_for_phv
(phv=0x55dbe14dc5c8,
    itlist=0x55dbe14e8a60, newvarno=-1, nrm_match=NRM_SUPERSET)
    at setrefs.c:2845
        subphv = 0x55dbe14e7120
        newvar = 0x0
        tle = 0x55dbe14e85f0
        lc__state = {l = 0x55dbe14e8640, i = 0}
        lc = 0x55dbe14e8658
#4  0x000055dbdffd82df in fix_join_expr_mutator (node=0x55dbe14dc5c8,
    context=0x7ffdf5c7c660) at setrefs.c:3066
        phv = 0x55dbe14dc5c8
        newvar = 0x0
        __func__ = "fix_join_expr_mutator"
#5  0x000055dbdff14963 in expression_tree_mutator_impl
(node=0x55dbe14e8840,
    mutator=0x55dbdffd80b7 <fix_join_expr_mutator>,
context=0x7ffdf5c7c660)
    at nodeFuncs.c:3051
        targetentry = 0x55dbe14e8840
        newnode = 0x55dbe14e9820
        __func__ = "expression_tree_mutator_impl"
#6  0x000055dbdffd843b in fix_join_expr_mutator (node=0x55dbe14e8840,
    context=0x7ffdf5c7c660) at setrefs.c:3104
        newvar = 0x15c31daf4ca7e00
        __func__ = "fix_join_expr_mutator"
#7  0x000055dbdff14d6e in expression_tree_mutator_impl
(node=0x55dbe14e8890,
    mutator=0x55dbdffd80b7 <fix_join_expr_mutator>,
context=0x7ffdf5c7c660)
    at nodeFuncs.c:3137
        temp__state = {l = 0x55dbe14e8890, i = 0}
        resultlist = 0x0
        temp = 0x55dbe14e88a8
        __func__ = "expression_tree_mutator_impl"
#8  0x000055dbdffd843b in fix_join_expr_mutator (node=0x55dbe14e8890,
    context=0x7ffdf5c7c660) at setrefs.c:3104
        newvar = 0x55dbe14e8a60
        __func__ = "fix_join_expr_mutator"
#9  0x000055dbdffd80a1 in fix_join_expr (root=0x55dbe14d4f80,
    clauses=0x55dbe14e8890, outer_itlist=0x55dbe14e8a10,
    inner_itlist=0x55dbe14e8a60, acceptable_rel=0, rtoffset=0,
    nrm_match=NRM_SUPERSET, num_exec=2910) at setrefs.c:2999
        context = {root = 0x55dbe14d4f80, outer_itlist = 0x55dbe14e8a10,
          inner_itlist = 0x55dbe14e8a60, acceptable_rel = 0, rtoffset = 0,
          nrm_match = NRM_SUPERSET, num_exec = 2910}
#10 0x000055dbdffd7111 in set_join_references (root=0x55dbe14d4f80,
    join=0x55dbe14e87b0, rtoffset=0) at setrefs.c:2350
        outer_plan = 0x55dbe14e8560
        inner_plan = 0x55dbe14e86f0
        outer_itlist = 0x55dbe14e8a10
        inner_itlist = 0x55dbe14e8a60
        __func__ = "set_join_references"
#11 0x000055dbdffd3ef9 in set_plan_refs (root=0x55dbe14d4f80,
    plan=0x55dbe14e87b0, rtoffset=0) at setrefs.c:870
        l = 0x55dbe1395090
        __func__ = "set_plan_refs"
#12 0x000055dbdffd2a1f in set_plan_references (root=0x55dbe14d4f80,
    plan=0x55dbe14e87b0) at setrefs.c:354
        result = 0x55dbe14dc108
        glob = 0x55dbe14d6678
        rtoffset = 0
        lc = 0x0
#13 0x000055dbdffc43cd in standard_planner (parse=0x55dbe14bd2c8,
    query_string=0x55dbe13951a8 "SELECT subq_3.c1 AS c8\nFROM t6 AS
sample_0\n     LEFT JOIN (n AS ref_1\n", ' ' <repeats 16 times>, "RIGHT JOIN
f AS sample_1 ON NULL\n", ' ' <repeats 16 times>, "LEFT JOIN (n2 AS
sample_3\n", ' ' <repeats 27 times>, "RIGHT JOIN t"..., cursorOptions=2048,
boundParams=0x0)
    at planner.c:504
        result = 0x0
        glob = 0x55dbe14d6678
        tuple_fraction = 0
        root = 0x55dbe14d4f80
        final_rel = 0x55dbe14e81f0
        best_path = 0x55dbe14e8050
        top_plan = 0x55dbe14e87b0
        lp = 0x4d430000
        lr = 0x55dbe14d55a0


Thanks to SQLSmith / SQLReduce for the find.

-
Robins Tharakan
Amazon Web Services

  2. Re: BUG #17781: Assert in setrefs.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-02-07T23:26:57Z

    PG Bug reporting form <noreply@postgresql.org> writes:
> This assert() is easily reproducible as of aa69541046@master, although I can
> trace the issue back to last week's commit 2489d76c49.

Pushed a fix, thanks!

			regards, tom lane

  3. Re: BUG #17781: Assert in setrefs.c

    Robins Tharakan <tharakan@gmail.com> — 2023-02-08T05:51:54Z

    Hi Tom,

Assuming this persistence is helpful overall, now I see a
different SQL can still trigger the same assert().

On Wed, 8 Feb 2023 at 09:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
> > This assert() is easily reproducible as of aa69541046@master, although I can
> > trace the issue back to last week's commit 2489d76c49.
>
> Pushed a fix, thanks!


TRAP: failed Assert("nrm_match == NRM_SUBSET ?
bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match
== NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels,
phv->phnullingrels) : bms_equal(subphv->phnullingrels,
phv->phnullingrels)"), File: "setrefs.c", Line: 2845, PID: 803757


SQL
===
rollback;
begin;
create table t();
SELECT ref_1.definition AS c4
FROM t AS sample_1
     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON NULL
WHERE pg_catalog."user"() IS NOT NULL;


Checking (fee7b77b90~0) - fee7b77b90 - Crash
Checking (fee7b77b90~1) - e2c78e7ab4 - Success
Checking (fee7b77b90~2) - 2cbbffff05 - Success


=== Backtrace - PID 803757 - fee7b77b90@master ===
Core was generated by `postgres: fee7b77b90@master@sqith: ubuntu t
127.0.0.1(60194) SELECT           '.
Program terminated with signal SIGABRT, Aborted.
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007f76d487e859 in __GI_abort () at abort.c:79
#2  0x000055c7f48e9126 in ExceptionalCondition (
    conditionName=0x55c7f4aa0ec0 "nrm_match == NRM_SUBSET ?
bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match
== NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels,
phv->phnullingrels) : bms_equal(subphv->phnullingr"...,
fileName=0x55c7f4aa08ef "setrefs.c", lineNumber=2845) at assert.c:66
#3  0x000055c7f45d8df7 in search_indexed_tlist_for_phv (phv=0x55c7f6bd3b68,
    itlist=0x55c7f6bdf988, newvarno=-2, nrm_match=NRM_EQUAL) at setrefs.c:2845
#4  0x000055c7f45d959c in fix_upper_expr_mutator (node=0x55c7f6bd3b68,
    context=0x7ffec773f9e0) at setrefs.c:3188
#5  0x000055c7f45d9482 in fix_upper_expr (root=0x55c7f6bcca28,
    node=0x55c7f6bd3b68, subplan_itlist=0x55c7f6bdf988, newvarno=-2,
    rtoffset=0, nrm_match=NRM_EQUAL, num_exec=2910) at setrefs.c:3158
#6  0x000055c7f45d8271 in set_upper_references (root=0x55c7f6bcca28,
    plan=0x55c7f6bdf6e8, rtoffset=0) at setrefs.c:2423
#7  0x000055c7f45d5406 in set_plan_refs (root=0x55c7f6bcca28,
    plan=0x55c7f6bdf6e8, rtoffset=0) at setrefs.c:1034
#8  0x000055c7f45d39ee in set_plan_references (root=0x55c7f6bcca28,
    plan=0x55c7f6bdf6e8) at setrefs.c:354
#9  0x000055c7f45c539c in standard_planner (parse=0x55c7f6a5f7f8,
    query_string=0x55c7f6a5e508 "SELECT ref_1.definition AS c4\nFROM t
AS sample_1\n     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON
NULL\nWHERE pg_catalog.\"user\"() IS NOT NULL;", cursorOptions=2048,
boundParams=0x0) at planner.c:504
#10 0x00007f76d186c08b in pgss_planner (parse=0x55c7f6a5f7f8,
    query_string=0x55c7f6a5e508 "SELECT ref_1.definition AS c4\nFROM t
AS sample_1\n     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON
NULL\nWHERE pg_catalog.\"user\"() IS NOT NULL;", cursorOptions=2048,
boundParams=0x0)
    at pg_stat_statements.c:954
#11 0x000055c7f45c4c3d in planner (parse=0x55c7f6a5f7f8,
    query_string=0x55c7f6a5e508 "SELECT ref_1.definition AS c4\nFROM t
AS sample_1\n     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON
NULL\nWHERE pg_catalog.\"user\"() IS NOT NULL;", cursorOptions=2048,
boundParams=0x0) at planner.c:279
#12 0x000055c7f4709d72 in pg_plan_query (querytree=0x55c7f6a5f7f8,
    query_string=0x55c7f6a5e508 "SELECT ref_1.definition AS c4\nFROM t
AS sample_1\n     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON
NULL\nWHERE pg_catalog.\"user\"() IS NOT NULL;", cursorOptions=2048,
boundParams=0x0) at postgres.c:870
#13 0x000055c7f4709eca in pg_plan_queries (querytrees=0x55c7f6bb4eb8,
    query_string=0x55c7f6a5e508 "SELECT ref_1.definition AS c4\nFROM t
AS sample_1\n     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON
NULL\nWHERE pg_catalog.\"user\"() IS NOT NULL;", cursorOptions=2048,
boundParams=0x0) at postgres.c:962
#14 0x000055c7f470a2ad in exec_simple_query (
    query_string=0x55c7f6a5e508 "SELECT ref_1.definition AS c4\nFROM t
AS sample_1\n     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON
NULL\nWHERE pg_catalog.\"user\"() IS NOT NULL;") at postgres.c:1156
#15 0x000055c7f470f163 in PostgresMain (dbname=0x55c7f6a59478 "t",
    username=0x55c7f6a9f5f8 "ubuntu") at postgres.c:4565
#16 0x000055c7f4637d4d in BackendRun (port=0x55c7f6a953d0)
    at postmaster.c:4461
#17 0x000055c7f4637620 in BackendStartup (port=0x55c7f6a953d0)
    at postmaster.c:4189
#18 0x000055c7f4633a4f in ServerLoop () at postmaster.c:1779
#19 0x000055c7f4633324 in PostmasterMain (argc=3, argv=0x55c7f6a573d0)
    at postmaster.c:1463
#20 0x000055c7f44e65eb in main (argc=3, argv=0x55c7f6a573d0) at main.c:200

=== Backtrace FULL - PID 803757 - fee7b77b90@master ===
#2  0x000055c7f48e9126 in ExceptionalCondition (
    conditionName=0x55c7f4aa0ec0 "nrm_match == NRM_SUBSET ?
bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match
== NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels,
phv->phnullingrels) : bms_equal(subphv->phnullingr"...,
fileName=0x55c7f4aa08ef "setrefs.c", lineNumber=2845) at assert.c:66
No locals.
#3  0x000055c7f45d8df7 in search_indexed_tlist_for_phv (phv=0x55c7f6bd3b68,
    itlist=0x55c7f6bdf988, newvarno=-2, nrm_match=NRM_EQUAL) at setrefs.c:2845
        subphv = 0x55c7f6bdeaa8
        newvar = 0x16e455bdfc6eef00
        tle = 0x55c7f6bdf1b8
        lc__state = {l = 0x55c7f6bdf208, i = 0}
        lc = 0x55c7f6bdf220
#4  0x000055c7f45d959c in fix_upper_expr_mutator (node=0x55c7f6bd3b68,
    context=0x7ffec773f9e0) at setrefs.c:3188
        phv = 0x55c7f6bd3b68
        newvar = 0x55c7f6a5e3f0
        __func__ = "fix_upper_expr_mutator"
#5  0x000055c7f45d9482 in fix_upper_expr (root=0x55c7f6bcca28,
    node=0x55c7f6bd3b68, subplan_itlist=0x55c7f6bdf988, newvarno=-2,
    rtoffset=0, nrm_match=NRM_EQUAL, num_exec=2910) at setrefs.c:3158
        context = {root = 0x55c7f6bcca28, subplan_itlist = 0x55c7f6bdf988,
          newvarno = -2, rtoffset = 0, nrm_match = NRM_EQUAL, num_exec = 2910}
#6  0x000055c7f45d8271 in set_upper_references (root=0x55c7f6bcca28,
    plan=0x55c7f6bdf6e8, rtoffset=0) at setrefs.c:2423
        tle = 0x55c7f6bdf778
        newexpr = 0x8
        l__state = {l = 0x55c7f6bdf7c8, i = 0}
        subplan = 0x55c7f6bdf508
        subplan_itlist = 0x55c7f6bdf988
        output_targetlist = 0x0
        l = 0x55c7f6bdf7e0
#7  0x000055c7f45d5406 in set_plan_refs (root=0x55c7f6bcca28,
    plan=0x55c7f6bdf6e8, rtoffset=0) at setrefs.c:1034
        splan = 0x55c7f6bdf6e8
        l = 0x55c7f6a5e3f0
        __func__ = "set_plan_refs"
#8  0x000055c7f45d39ee in set_plan_references (root=0x55c7f6bcca28,
    plan=0x55c7f6bdf6e8) at setrefs.c:354
        result = 0x55c7f6bd3b18
        glob = 0x55c7f6bcbea8
        rtoffset = 0
        lc = 0x0
#9  0x000055c7f45c539c in standard_planner (parse=0x55c7f6a5f7f8,
    query_string=0x55c7f6a5e508 "SELECT ref_1.definition AS c4\nFROM t
AS sample_1\n     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON
NULL\nWHERE pg_catalog.\"user\"() IS NOT NULL;", cursorOptions=2048,
boundParams=0x0) at planner.c:504
        result = 0x0
        glob = 0x55c7f6bcbea8
        tuple_fraction = 0
        root = 0x55c7f6bcca28
        final_rel = 0x55c7f6bdd948
        best_path = 0x55c7f6bdef38
        top_plan = 0x55c7f6bdf6e8
        lp = 0x4d430000
        lr = 0x55c7f6bb4eb8



2023-02-08 01:35:21.274 UTC [4051201] LOG:  server process (PID
803757) was terminated by signal 6: Aborted
2023-02-08 01:35:21.274 UTC [4051201] DETAIL:  Failed process was
running: SELECT ref_1.definition AS c4
FROM t AS sample_1
     LEFT JOIN pg_catalog.pg_rules AS ref_1 ON NULL
WHERE pg_catalog."user"() IS NOT NULL;
2023-02-08 01:35:21.274 UTC [4051201] LOG:  terminating any other
active server processes

As earlier, thanks to SQLSmith / SQLReduce.

-
Robins Tharakan
Amazon Web Services

  4. Re: BUG #17781: Assert in setrefs.c

    Richard Guo <guofenglinux@gmail.com> — 2023-02-08T07:02:16Z

    On Wed, Feb 8, 2023 at 1:52 PM Robins Tharakan <tharakan@gmail.com> wrote:

> Hi Tom,
>
> Assuming this persistence is helpful overall, now I see a
> different SQL can still trigger the same assert().
>
> On Wed, 8 Feb 2023 at 09:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > PG Bug reporting form <noreply@postgresql.org> writes:
> > > This assert() is easily reproducible as of aa69541046@master,
> although I can
> > > trace the issue back to last week's commit 2489d76c49.
> >
> > Pushed a fix, thanks!
>
>
> TRAP: failed Assert("nrm_match == NRM_SUBSET ?
> bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match
> == NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels,
> phv->phnullingrels) : bms_equal(subphv->phnullingrels,
> phv->phnullingrels)"), File: "setrefs.c", Line: 2845, PID: 803757
>
>
> SQL
> ===
> rollback;
> begin;
> create table t();
> SELECT ref_1.definition AS c4
> FROM t AS sample_1
>      LEFT JOIN pg_catalog.pg_rules AS ref_1 ON NULL
> WHERE pg_catalog."user"() IS NOT NULL;


I've looked at this a little bit and I think it's a different issue.  It
seems when we've decided that a left join can be removed, we neglect to
remove references to the target rel from PlaceHolderVar->phrels in
remove_rel_from_query.  And it turns out that PlaceHolderVar->phrels is
used later in build_joinrel_tlist to check whether the PHV actually
comes from the nullable side of an outer join.  I verified that with the
following change and it seems can fix this query.

--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -429,11 +429,16 @@ remove_rel_from_query(PlannerInfo *root, int relid,
int ojrelid,
        }
        else
        {
+           PlaceHolderVar  *phv = phinfo->ph_var;
+
            phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
            phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at,
ojrelid);
            Assert(!bms_is_empty(phinfo->ph_eval_at));
            phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
            phinfo->ph_needed = bms_del_member(phinfo->ph_needed, ojrelid);
+
+           phv->phrels = bms_del_member(phv->phrels, relid);
+           phv->phrels = bms_del_member(phv->phrels, ojrelid);
        }

Thanks
Richard

  5. Re: BUG #17781: Assert in setrefs.c

    Richard Guo <guofenglinux@gmail.com> — 2023-02-08T09:15:24Z

    On Wed, Feb 8, 2023 at 3:02 PM Richard Guo <guofenglinux@gmail.com> wrote:

>
> On Wed, Feb 8, 2023 at 1:52 PM Robins Tharakan <tharakan@gmail.com> wrote:
>
>> Hi Tom,
>>
>> Assuming this persistence is helpful overall, now I see a
>> different SQL can still trigger the same assert().
>>
>> On Wed, 8 Feb 2023 at 09:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> > PG Bug reporting form <noreply@postgresql.org> writes:
>> > > This assert() is easily reproducible as of aa69541046@master,
>> although I can
>> > > trace the issue back to last week's commit 2489d76c49.
>> >
>> > Pushed a fix, thanks!
>>
>>
>> TRAP: failed Assert("nrm_match == NRM_SUBSET ?
>> bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match
>> == NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels,
>> phv->phnullingrels) : bms_equal(subphv->phnullingrels,
>> phv->phnullingrels)"), File: "setrefs.c", Line: 2845, PID: 803757
>>
>>
>> SQL
>> ===
>> rollback;
>> begin;
>> create table t();
>> SELECT ref_1.definition AS c4
>> FROM t AS sample_1
>>      LEFT JOIN pg_catalog.pg_rules AS ref_1 ON NULL
>> WHERE pg_catalog."user"() IS NOT NULL;
>
>
> I've looked at this a little bit and I think it's a different issue.  It
> seems when we've decided that a left join can be removed, we neglect to
> remove references to the target rel from PlaceHolderVar->phrels in
> remove_rel_from_query.  And it turns out that PlaceHolderVar->phrels is
> used later in build_joinrel_tlist to check whether the PHV actually
> comes from the nullable side of an outer join.  I verified that with the
> following change and it seems can fix this query.
>
> --- a/src/backend/optimizer/plan/analyzejoins.c
> +++ b/src/backend/optimizer/plan/analyzejoins.c
> @@ -429,11 +429,16 @@ remove_rel_from_query(PlannerInfo *root, int relid,
> int ojrelid,
>         }
>         else
>         {
> +           PlaceHolderVar  *phv = phinfo->ph_var;
> +
>             phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
>             phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at,
> ojrelid);
>             Assert(!bms_is_empty(phinfo->ph_eval_at));
>             phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
>             phinfo->ph_needed = bms_del_member(phinfo->ph_needed, ojrelid);
> +
> +           phv->phrels = bms_del_member(phv->phrels, relid);
> +           phv->phrels = bms_del_member(phv->phrels, ojrelid);
>         }
>

Hmm. I begin to wonder if it's better to use phinfo->ph_eval_at instead
in build_joinrel_tlist when we check whether the PHV actually comes from
the nullable side of an outer join.

--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -1095,9 +1095,9 @@ build_joinrel_tlist(PlannerInfo *root, RelOptInfo
*joinrel,
       phv = copyObject(phv);
       /* See comments above to understand this logic */
       if (sjinfo->ojrelid != 0 &&
-          (bms_is_subset(phv->phrels, sjinfo->syn_righthand) ||
+          (bms_is_subset(phinfo->ph_eval_at, sjinfo->syn_righthand) ||
            (sjinfo->jointype == JOIN_FULL &&
-            bms_is_subset(phv->phrels, sjinfo->syn_lefthand))))
+            bms_is_subset(phinfo->ph_eval_at, sjinfo->syn_lefthand))))

Even so it seems we still need to update phv->phrels in
remove_rel_from_query when we remove a left join.  Otherwise it'd be
weird to observe that phrels contains some already-removed relids.

Thanks
Richard

  6. Re: BUG #17781: Assert in setrefs.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-02-08T17:11:52Z

    Richard Guo <guofenglinux@gmail.com> writes:
> Hmm. I begin to wonder if it's better to use phinfo->ph_eval_at instead
> in build_joinrel_tlist when we check whether the PHV actually comes from
> the nullable side of an outer join.

I wondered about that too, but I think phrels is probably preferable,
because it is fundamentally a syntactic attribute and what we are
after here is matching the syntactic structure.  I could be convinced
that that's wrong, but I'd want to see an actual counterexample.

> Even so it seems we still need to update phv->phrels in
> remove_rel_from_query when we remove a left join.  Otherwise it'd be
> weird to observe that phrels contains some already-removed relids.

Agreed, that's a good catch.  As the comment for remove_rel_from_query
says,

 * We are not terribly thorough here.

It's not surprising that the requirements for it advance over time.

(At one point I wrote some code that taught it to clean out the
removed relids from EquivalenceClasses, which doesn't need
to happen right now because we don't ever put outer-join quals
into EquivalenceClasses.  I don't think I included that in any
of the published versions of the patch series, but it's going to
need to reappear eventually.)

			regards, tom lane

  7. Re: BUG #17781: Assert in setrefs.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-02-08T19:46:27Z

    Robins Tharakan <tharakan@gmail.com> writes:
> Assuming this persistence is helpful overall, now I see a
> different SQL can still trigger the same assert().

It's very helpful, thank you!  I pushed Richard's fix for this.

			regards, tom lane

  8. Re: BUG #17781: Assert in setrefs.c

    Robins Tharakan <tharakan@gmail.com> — 2023-02-10T06:50:06Z

    This assert() is still reproducible (as of 40d0b2d415@master) although
intermittent when trying to reach manually, in that I could reduce to
a simpler crash but after a while the crashes stop (maybe it's just
lack of coffee). Owing to this, this time I only provide table-data
instead of a simplified repro.

What I can confirm is that the test setup easily steps on this every
few minutes, so I'd say this still needs attention.


TRAP: failed Assert("varattno <= 0 || (nrm_match == NRM_SUBSET ?
bms_is_subset(var->varnullingrels, vinfo->varnullingrels) : nrm_match
== NRM_SUPERSET ? bms_is_subset(vinfo->varnullingrels,
var->varnullingrels) : bms_equal(vinfo->varnullingrels,
var->varnullingrels))"), File: "setrefs.c", Line: 2792, PID: 2005079


2023-02-10 00:28:34.886 UTC [1798150] LOG:  server process (PID
2005079) was terminated by signal 6: Aborted
2023-02-10 00:28:34.886 UTC [1798150] DETAIL:  Failed process was
running: SELECT
FROM (SELECT NULL AS c1
      LIMIT ALL) AS subq_2
     RIGHT JOIN public.caster AS ref_3 ON ref_3.int8 > ref_3.int2
     LEFT JOIN (SELECT
                LIMIT ALL) AS subq_3 ON ref_3.name !~ subq_2.c1
     RIGHT JOIN testxmlschema.test3 AS sample_2 ON ref_3.boolean = sample_2.c1


postgres=# select int2 is null, int8 is null, boolean is null,
count(*) from caster group by 1,2,3;
 ?column? | ?column? | ?column? | count
----------+----------+----------+-------
 t        | t        | t        |    80
(1 row)

postgres=# \d testxmlschema.test3
                 Table "testxmlschema.test3"
 Column |       Type        | Collation | Nullable | Default
--------+-------------------+-----------+----------+---------
 c1     | boolean           |           |          |
 c2     | testboolxmldomain |           |          |
 c3     | date              |           |          |
 c4     | testdatexmldomain |           |          |

postgres=# table testxmlschema.test3 ;
 c1 | c2 |     c3     |     c4
----+----+------------+------------
 t  | t  | 2013-02-21 | 2013-02-21
(1 row)






=== Backtrace - PID 2005079 - 40d0b2d415@master ===
Core was generated by `postgres: 40d0b2d415@master@sqith: ubuntu
postgres 127.0.0.1(39074) SELECT    '.
Program terminated with signal SIGABRT, Aborted.
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007fbd65b54859 in __GI_abort () at abort.c:79
#2  0x000055da10f04d9d in ExceptionalCondition (
    conditionName=0x55da110bcde8 "varattno <= 0 || (nrm_match ==
NRM_SUBSET ? bms_is_subset(var->varnullingrels, vinfo->varnullingrels)
: nrm_match == NRM_SUPERSET ? bms_is_subset(vinfo->varnullingrels,
var->varnullingrels) : bms_equa"..., fileName=0x55da110bc90f
"setrefs.c", lineNumber=2792) at assert.c:66
#3  0x000055da10bf3d35 in search_indexed_tlist_for_var (var=0x55da123a4178,
    itlist=0x55da123adeb0, newvarno=-2, rtoffset=0, nrm_match=NRM_EQUAL)
    at setrefs.c:2792
#4  0x000055da10bf419e in fix_join_expr_mutator (node=0x55da123a4178,
    context=0x7ffe3bd446a0) at setrefs.c:3016
#5  0x000055da10b30d6e in expression_tree_mutator_impl (node=0x55da123a3e78,
    mutator=0x55da10bf4129 <fix_join_expr_mutator>, context=0x7ffe3bd446a0)
    at nodeFuncs.c:3137
#6  0x000055da10bf44ad in fix_join_expr_mutator (node=0x55da123a3e78,
    context=0x7ffe3bd446a0) at setrefs.c:3104
#7  0x000055da10b2fbd9 in expression_tree_mutator_impl (node=0x55da123a40b8,
    mutator=0x55da10bf4129 <fix_join_expr_mutator>, context=0x7ffe3bd446a0)
    at nodeFuncs.c:2771
#8  0x000055da10bf44ad in fix_join_expr_mutator (node=0x55da123a40b8,
    context=0x7ffe3bd446a0) at setrefs.c:3104
#9  0x000055da10b30d6e in expression_tree_mutator_impl (node=0x55da123ad820,
    mutator=0x55da10bf4129 <fix_join_expr_mutator>, context=0x7ffe3bd446a0)
    at nodeFuncs.c:3137
#10 0x000055da10bf44ad in fix_join_expr_mutator (node=0x55da123ad820,
    context=0x7ffe3bd446a0) at setrefs.c:3104
#11 0x000055da10bf4113 in fix_join_expr (root=0x55da123949f0,
    clauses=0x55da123ad820, outer_itlist=0x55da123adeb0,
    inner_itlist=0x55da123ae080, acceptable_rel=0, rtoffset=0,
    nrm_match=NRM_EQUAL, num_exec=2) at setrefs.c:2999
#12 0x000055da10bf2ee3 in set_join_references (root=0x55da123949f0,
    join=0x55da123ad850, rtoffset=0) at setrefs.c:2260
#13 0x000055da10beff6b in set_plan_refs (root=0x55da123949f0,
    plan=0x55da123ad850, rtoffset=0) at setrefs.c:870
#14 0x000055da10beea91 in set_plan_references (root=0x55da123949f0,
    plan=0x55da123ad850) at setrefs.c:354
#15 0x000055da10be043f in standard_planner (parse=0x55da1229de28,
    query_string=0x55da1226a508 "SELECT\nFROM (SELECT NULL AS c1\n
 LIMIT ALL) AS subq_2\n     RIGHT JOIN public.caster AS ref_3 ON
ref_3.int8 > ref_3.int2\n     LEFT JOIN (SELECT\n", ' ' <repeats 16
times>, "LIMIT ALL) AS subq_3 ON ref_3.name !~"...,
cursorOptions=2048, boundParams=0x0) at planner.c:504
#16 0x00007fbd62b4208b in pgss_planner (parse=0x55da1229de28,
    query_string=0x55da1226a508 "SELECT\nFROM (SELECT NULL AS c1\n
 LIMIT ALL) AS subq_2\n     RIGHT JOIN public.caster AS ref_3 ON
ref_3.int8 > ref_3.int2\n     LEFT JOIN (SELECT\n", ' ' <repeats 16
times>, "LIMIT ALL) AS subq_3 ON ref_3.name !~"...,
cursorOptions=2048, boundParams=0x0)
    at pg_stat_statements.c:954
#17 0x000055da10bdfce0 in planner (parse=0x55da1229de28,
    query_string=0x55da1226a508 "SELECT\nFROM (SELECT NULL AS c1\n
 LIMIT ALL) AS subq_2\n     RIGHT JOIN public.caster AS ref_3 ON
ref_3.int8 > ref_3.int2\n     LEFT JOIN (SELECT\n", ' ' <repeats 16
times>, "LIMIT ALL) AS subq_3 ON ref_3.name !~"...,
cursorOptions=2048, boundParams=0x0) at planner.c:279
#18 0x000055da10d24f88 in pg_plan_query (querytree=0x55da1229de28,
    query_string=0x55da1226a508 "SELECT\nFROM (SELECT NULL AS c1\n
 LIMIT ALL) AS subq_2\n     RIGHT JOIN public.caster AS ref_3 ON
ref_3.int8 > ref_3.int2\n     LEFT JOIN (SELECT\n", ' ' <repeats 16
times>, "LIMIT ALL) AS subq_3 ON ref_3.name !~"...,
cursorOptions=2048, boundParams=0x0) at postgres.c:870
#19 0x000055da10d250e0 in pg_plan_queries (querytrees=0x55da123949c0,
    query_string=0x55da1226a508 "SELECT\nFROM (SELECT NULL AS c1\n
 LIMIT ALL) AS subq_2\n     RIGHT JOIN public.caster AS ref_3 ON
ref_3.int8 > ref_3.int2\n     LEFT JOIN (SELECT\n", ' ' <repeats 16
times>, "LIMIT ALL) AS subq_3 ON ref_3.name !~"...,
cursorOptions=2048, boundParams=0x0) at postgres.c:962
#20 0x000055da10d254c3 in exec_simple_query (
    query_string=0x55da1226a508 "SELECT\nFROM (SELECT NULL AS c1\n
 LIMIT ALL) AS subq_2\n     RIGHT JOIN public.caster AS ref_3 ON
ref_3.int8 > ref_3.int2\n     LEFT JOIN (SELECT\n", ' ' <repeats 16
times>, "LIMIT ALL) AS subq_3 ON ref_3.name !~"...) at postgres.c:1156
#21 0x000055da10d2a379 in PostgresMain (dbname=0x55da122aa2e8 "postgres",
    username=0x55da12265478 "ubuntu") at postgres.c:4565
#22 0x000055da10c52dd6 in BackendRun (port=0x55da122a72b0)
    at postmaster.c:4461
#23 0x000055da10c526a9 in BackendStartup (port=0x55da122a72b0)
    at postmaster.c:4189
#24 0x000055da10c4ead8 in ServerLoop () at postmaster.c:1779
#25 0x000055da10c4e3ad in PostmasterMain (argc=3, argv=0x55da122633d0)
    at postmaster.c:1463
#26 0x000055da10b015eb in main (argc=3, argv=0x55da122633d0) at main.c:200



=== Backtrace FULL - PID 2005079 - 40d0b2d415@master ===
#2  0x000055da10f04d9d in ExceptionalCondition (
    conditionName=0x55da110bcde8 "varattno <= 0 || (nrm_match ==
NRM_SUBSET ? bms_is_subset(var->varnullingrels, vinfo->varnullingrels)
: nrm_match == NRM_SUPERSET ? bms_is_subset(vinfo->varnullingrels,
var->varnullingrels) : bms_equa"..., fileName=0x55da110bc90f
"setrefs.c", lineNumber=2792) at assert.c:66
No locals.
#3  0x000055da10bf3d35 in search_indexed_tlist_for_var (var=0x55da123a4178,
    itlist=0x55da123adeb0, newvarno=-2, rtoffset=0, nrm_match=NRM_EQUAL)
    at setrefs.c:2792
        newvar = 0x55da123ae140
        varno = 1
        varattno = 1
        vinfo = 0x55da123aded0
        i = 0
#4  0x000055da10bf419e in fix_join_expr_mutator (node=0x55da123a4178,
    context=0x7ffe3bd446a0) at setrefs.c:3016
        var = 0x55da123a4178
        newvar = 0x55da10b2690d <lappend+133>
        __func__ = "fix_join_expr_mutator"
#5  0x000055da10b30d6e in expression_tree_mutator_impl (node=0x55da123a3e78,
    mutator=0x55da10bf4129 <fix_join_expr_mutator>, context=0x7ffe3bd446a0)
    at nodeFuncs.c:3137
        temp__state = {l = 0x55da123a3e78, i = 1}
        resultlist = 0x55da123adc10
        temp = 0x55da123a41d0
        __func__ = "expression_tree_mutator_impl"
#6  0x000055da10bf44ad in fix_join_expr_mutator (node=0x55da123a3e78,
    context=0x7ffe3bd446a0) at setrefs.c:3104
        newvar = 0x0
        __func__ = "fix_join_expr_mutator"
#7  0x000055da10b2fbd9 in expression_tree_mutator_impl (node=0x55da123a40b8,
    mutator=0x55da10bf4129 <fix_join_expr_mutator>, context=0x7ffe3bd446a0)
    at nodeFuncs.c:2771
        expr = 0x55da123a40b8
        newnode = 0x55da123ae0a0
        __func__ = "expression_tree_mutator_impl"
#8  0x000055da10bf44ad in fix_join_expr_mutator (node=0x55da123a40b8,
    context=0x7ffe3bd446a0) at setrefs.c:3104
        newvar = 0xe0e36faee4a53e00
        __func__ = "fix_join_expr_mutator"
#9  0x000055da10b30d6e in expression_tree_mutator_impl (node=0x55da123ad820,
    mutator=0x55da10bf4129 <fix_join_expr_mutator>, context=0x7ffe3bd446a0)
    at nodeFuncs.c:3137
        temp__state = {l = 0x55da123ad820, i = 0}
        resultlist = 0x0
        temp = 0x55da123ad838
        __func__ = "expression_tree_mutator_impl"
#10 0x000055da10bf44ad in fix_join_expr_mutator (node=0x55da123ad820,
    context=0x7ffe3bd446a0) at setrefs.c:3104
        newvar = 0x55da123ae080
        __func__ = "fix_join_expr_mutator"
#11 0x000055da10bf4113 in fix_join_expr (root=0x55da123949f0,
    clauses=0x55da123ad820, outer_itlist=0x55da123adeb0,
    inner_itlist=0x55da123ae080, acceptable_rel=0, rtoffset=0,
    nrm_match=NRM_EQUAL, num_exec=2) at setrefs.c:2999
        context = {root = 0x55da123949f0, outer_itlist = 0x55da123adeb0,
          inner_itlist = 0x55da123ae080, acceptable_rel = 0, rtoffset = 0,
          nrm_match = NRM_EQUAL, num_exec = 2}
#12 0x000055da10bf2ee3 in set_join_references (root=0x55da123949f0,
    join=0x55da123ad850, rtoffset=0) at setrefs.c:2260
        outer_plan = 0x55da123ad670
        inner_plan = 0x55da123ad790
        outer_itlist = 0x55da123adeb0
        inner_itlist = 0x55da123ae080
        __func__ = "set_join_references"
#13 0x000055da10beff6b in set_plan_refs (root=0x55da123949f0,
    plan=0x55da123ad850, rtoffset=0) at setrefs.c:870
        l = 0x55da1226a3f0
        __func__ = "set_plan_refs"
#14 0x000055da10beea91 in set_plan_references (root=0x55da123949f0,
    plan=0x55da123ad850) at setrefs.c:354
        result = 0x55da10bcbe95 <create_plan_recurse+134>
        glob = 0x55da1226c2d8
        rtoffset = 0
        lc = 0x0
#15 0x000055da10be043f in standard_planner (parse=0x55da1229de28,
    query_string=0x55da1226a508 "SELECT\nFROM (SELECT NULL AS c1\n
 LIMIT ALL) AS subq_2\n     RIGHT JOIN public.caster AS ref_3 ON
ref_3.int8 > ref_3.int2\n     LEFT JOIN (SELECT\n", ' ' <repeats 16
times>, "LIMIT ALL) AS subq_3 ON ref_3.name !~"...,
cursorOptions=2048, boundParams=0x0) at planner.c:504
        result = 0x55da10ee048d <SearchCatCacheInternal+607>
        glob = 0x55da1226c2d8
        tuple_fraction = 0
        root = 0x55da123949f0
        final_rel = 0x55da123ab050
        best_path = 0x55da123aaba0
        top_plan = 0x55da123ad850
        lp = 0x0
        lr = 0x0


-
Robins Tharakan
Amazon Web Services

  9. Re: BUG #17781: Assert in setrefs.c

    Robins Tharakan <tharakan@gmail.com> — 2023-02-10T07:34:36Z

    On Fri, 10 Feb 2023 at 17:20, Robins Tharakan <tharakan@gmail.com> wrote:
>
> What I can confirm is that the test setup easily steps on this every
> few minutes, so I'd say this still needs attention.

My bad. I should have said every hour or so.

-
robins