Horton

Thread

Commits

  1. Remove bogus Assert and dead code in remove_useless_results_recurse().

  1. BUG #17700: An assert failed in prepjointree.c

    The Post Office <noreply@postgresql.org> — 2022-11-28T09:09:33Z

    The following bug has been logged on the website:

Bug reference:      17700
Logged by:          xin wen
Email address:      xinwen@stu.scu.edu.cn
PostgreSQL version: 15.1
Operating system:   Ubuntu 20.04
Description:        

When executing the following query:

WITH table3 ( column34 ) AS NOT MATERIALIZED ( SELECT 1 ) SELECT 1 FROM (
SELECT 1 FROM table3 WHERE column34 = ANY ( SELECT alias4 . column34 FROM
table3 FULL JOIN table3 AS alias4 ON TRUE )) AS alias5 ;

I get a failed assertion with the following stacktrace:

Core was generated by `postgres: postgres postgres [local] SELECT           
'.
Program terminated with signal SIGABRT, Aborted.
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ff7f1a36859 in __GI_abort () at abort.c:79
#2  0x0000563be15a2a88 in ExceptionalCondition
(conditionName=conditionName@entry=0x563be1713818
"!find_dependent_phvs(root, varno)",
errorType=errorType@entry=0x563be1600498 "FailedAssertion", 
    fileName=fileName@entry=0x563be1713580
"/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/prep/prepjointree.c",
lineNumber=lineNumber@entry=3301)
    at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/utils/error/assert.c:69
#3  0x0000563be13b15cb in remove_useless_results_recurse
(root=0x563be32a6cb8, jtnode=0x563be32a6158) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/prep/prepjointree.c:3301
#4  0x0000563be13b1304 in remove_useless_results_recurse
(root=0x563be32a6cb8, jtnode=0x563be32a6bd0) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/prep/prepjointree.c:3155
#5  0x0000563be13b15f6 in remove_useless_result_rtes
(root=root@entry=0x563be32a6cb8) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/prep/prepjointree.c:3095
#6  0x0000563be13a4fca in subquery_planner (glob=glob@entry=0x563be32a6000,
parse=parse@entry=0x563be31dee50, parent_root=parent_root@entry=0x0,
hasRecursion=hasRecursion@entry=false, 
    tuple_fraction=tuple_fraction@entry=0) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/plan/planner.c:1039
#7  0x0000563be13a52a3 in standard_planner (parse=0x563be31dee50,
query_string=<optimized out>, cursorOptions=2048, boundParams=<optimized
out>)
    at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/plan/planner.c:406
#8  0x0000563be1476edc in pg_plan_query (querytree=0x563be31dee50, 
    query_string=0x563be31dd070 "WITH table3 ( column34 ) AS NOT
MATERIALIZED ( SELECT 1 ) SELECT 1 FROM ( SELECT 1 FROM table3 WHERE
column34 = ANY ( SELECT alias4 . column34 FROM table3 FULL JOIN table3 AS
alias4 ON TRUE )) AS alia"..., cursorOptions=2048, boundParams=0x0) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/tcop/postgres.c:883
#9  0x0000563be1476fd1 in pg_plan_queries (querytrees=0x563be32a6c60, 
    query_string=query_string@entry=0x563be31dd070 "WITH table3 ( column34 )
AS NOT MATERIALIZED ( SELECT 1 ) SELECT 1 FROM ( SELECT 1 FROM table3 WHERE
column34 = ANY ( SELECT alias4 . column34 FROM table3 FULL JOIN table3 AS
alias4 ON TRUE )) AS alia"..., cursorOptions=cursorOptions@entry=2048,
boundParams=boundParams@entry=0x0)
    at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/tcop/postgres.c:975
#10 0x0000563be14773df in exec_simple_query (
    query_string=0x563be31dd070 "WITH table3 ( column34 ) AS NOT
MATERIALIZED ( SELECT 1 ) SELECT 1 FROM ( SELECT 1 FROM table3 WHERE
column34 = ANY ( SELECT alias4 . column34 FROM table3 FULL JOIN table3 AS
alias4 ON TRUE )) AS alia"...) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/tcop/postgres.c:1169
#11 0x0000563be1478f8c in PostgresMain (dbname=<optimized out>,
username=<optimized out>) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/tcop/postgres.c:4581
#12 0x0000563be13e5e8a in BackendRun (port=<optimized out>, port=<optimized
out>) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/postmaster/postmaster.c:4504
#13 BackendStartup (port=<optimized out>) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/postmaster/postmaster.c:4232
#14 ServerLoop () at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/postmaster/postmaster.c:1806
#15 0x0000563be13e6ffb in PostmasterMain (argc=<optimized out>,
argv=0x563be31d7370) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/postmaster/postmaster.c:1478
#16 0x0000563be1111630 in main (argc=3, argv=0x563be31d7370) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/main/main.c:202

I also find this assert failed in 12.13, 13.9 and 14.6 using the same
statement.

  2. Re: BUG #17700: An assert failed in prepjointree.c

    Richard Guo <guofenglinux@gmail.com> — 2022-11-28T12:32:15Z

    On Mon, Nov 28, 2022 at 5:27 PM PG Bug reporting form <
noreply@postgresql.org> wrote:

> WITH table3 ( column34 ) AS NOT MATERIALIZED ( SELECT 1 ) SELECT 1 FROM (
> SELECT 1 FROM table3 WHERE column34 = ANY ( SELECT alias4 . column34 FROM
> table3 FULL JOIN table3 AS alias4 ON TRUE )) AS alias5 ;


Thanks for the report! I can reproduce this issue on HEAD.

I haven't got too much time looking into it.  But the comment near the
assertion failure that says

 * Unlike the LEFT/RIGHT cases, we just Assert that there are
 * no PHVs that need to be evaluated at the semijoin's RHS,
 * since the rest of the query couldn't reference any outputs
 * of the semijoin's RHS.

I doubt this is true as a semijoin's qual can actually reference its
RHS.  In this case the assertion failure happens because there is PHV in
the join's qual.

I tried the change as to also check for PHVs that have to be evaluated
in the semijoin's RHS, like how we do for left/right join, and it can
avoid the assertion failure.  But I'm not sure if this is a reasonable
fix.

Thanks
Richard

  3. Re: BUG #17700: An assert failed in prepjointree.c

    Daniel Gustafsson <daniel@yesql.se> — 2022-11-28T12:32:45Z

    > On 28 Nov 2022, at 10:09, PG Bug reporting form <noreply@postgresql.org> wrote:

> When executing the following query:
> 
> WITH table3 ( column34 ) AS NOT MATERIALIZED ( SELECT 1 ) SELECT 1 FROM (
> SELECT 1 FROM table3 WHERE column34 = ANY ( SELECT alias4 . column34 FROM
> table3 FULL JOIN table3 AS alias4 ON TRUE )) AS alias5 ;

The full join on the rhs of the semi join creates a PlaceHolderVar due to
table3 not being materialized. A slightly shorter repro is:

with table3 as not materialized (
  select 1 as column34
)
select 1 from table3
where column34 in (
  select alias4.column34 from table3
    full join table3 as alias4 on true
);

--
Daniel Gustafsson		https://vmware.com/

  4. Re: BUG #17700: An assert failed in prepjointree.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-28T15:33:37Z

    Richard Guo <guofenglinux@gmail.com> writes:
> I haven't got too much time looking into it.  But the comment near the
> assertion failure that says

>  * Unlike the LEFT/RIGHT cases, we just Assert that there are
>  * no PHVs that need to be evaluated at the semijoin's RHS,
>  * since the rest of the query couldn't reference any outputs
>  * of the semijoin's RHS.

> I doubt this is true as a semijoin's qual can actually reference its
> RHS.  In this case the assertion failure happens because there is PHV in
> the join's qual.

Yeah, the possibility of a PHV in the qual refutes that argument.
So we need to make the test honestly, as in the other arms of that
switch.  Will fix.

			regards, tom lane

  5. Re: BUG #17700: An assert failed in prepjointree.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-28T16:22:07Z

    I wrote:
> Yeah, the possibility of a PHV in the qual refutes that argument.
> So we need to make the test honestly, as in the other arms of that
> switch.  Will fix.

Actually ... it seems like we could just drop that Assert, as per
revised argument in the comment.  This'd explain the lack of field
complaints: there's no bug in a production build.

			regards, tom lane

  6. Re: BUG #17700: An assert failed in prepjointree.c

    Richard Guo <guofenglinux@gmail.com> — 2022-11-29T00:12:16Z

    On Tue, Nov 29, 2022 at 12:22 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Actually ... it seems like we could just drop that Assert, as per
> revised argument in the comment.  This'd explain the lack of field
> complaints: there's no bug in a production build.


Ah yes, that's right.  We can be sure that above the semijoin there
would be no reference to its RHS.  And the PHV in semijoin's qual seems
unnecessary.

This reminds me of another question I had about unwrapping unnecessary
PHVs [1].  Sad that we don't have infrastructure for that.

[1]
https://www.postgresql.org/message-id/flat/CAMbWs4--Qy-nLq_Eq61_rsQ3JUYteNh2G8-GnDkt%2BFsp_t_Wjw%40mail.gmail.com

Thanks
Richard

  7. Re: BUG #17700: An assert failed in prepjointree.c

    Richard Guo <guofenglinux@gmail.com> — 2022-11-29T02:19:00Z

    On Tue, Nov 29, 2022 at 8:12 AM Richard Guo <guofenglinux@gmail.com> wrote:

> On Tue, Nov 29, 2022 at 12:22 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Actually ... it seems like we could just drop that Assert, as per
>> revised argument in the comment.  This'd explain the lack of field
>> complaints: there's no bug in a production build.
>
>
> Ah yes, that's right.  We can be sure that above the semijoin there
> would be no reference to its RHS.  And the PHV in semijoin's qual seems
> unnecessary.
>
> This reminds me of another question I had about unwrapping unnecessary
> PHVs [1].  Sad that we don't have infrastructure for that.
>

BTW, for the test case

+explain (verbose, costs off)
+with ctetable as not materialized ( select 1 as f1 )
+select * from ctetable c1
+where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on true
);

Actually we just need to keep 'c3' in a join's nullable side to have the
PHV created.  So we don't have to use full join in the subquery.  A left
join would do.

Thanks
Richard

  8. Re: BUG #17700: An assert failed in prepjointree.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-29T02:24:13Z

    Richard Guo <guofenglinux@gmail.com> writes:
> BTW, for the test case

> +explain (verbose, costs off)
> +with ctetable as not materialized ( select 1 as f1 )
> +select * from ctetable c1
> +where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on true
> );

> Actually we just need to keep 'c3' in a join's nullable side to have the
> PHV created.  So we don't have to use full join in the subquery.  A left
> join would do.

Actually, the planner reduces the full join to left join anyway;
if it did not, it wouldn't be able to reach the code in question.
I think this formulation is fine because it tests that step along
with the bug proper.

			regards, tom lane

  9. Re: BUG #17700: An assert failed in prepjointree.c

    Richard Guo <guofenglinux@gmail.com> — 2022-11-29T03:12:02Z

    On Tue, Nov 29, 2022 at 10:24 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Richard Guo <guofenglinux@gmail.com> writes:
> > BTW, for the test case
>
> > +explain (verbose, costs off)
> > +with ctetable as not materialized ( select 1 as f1 )
> > +select * from ctetable c1
> > +where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on
> true
> > );
>
> > Actually we just need to keep 'c3' in a join's nullable side to have the
> > PHV created.  So we don't have to use full join in the subquery.  A left
> > join would do.
>
> Actually, the planner reduces the full join to left join anyway;
> if it did not, it wouldn't be able to reach the code in question.
> I think this formulation is fine because it tests that step along
> with the bug proper.


Hmm, I see your point.  You're right.  'c2 fulljoin c3' would be reduced
to 'c3 leftjoin c2'.

BTW, I wonder if we need the JOIN_RIGHT case here since JOIN_RIGHT
should have been flipped around to become JOIN_LEFT.

Thanks
Richard

  10. Re: BUG #17700: An assert failed in prepjointree.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-29T03:31:19Z

    Richard Guo <guofenglinux@gmail.com> writes:
> BTW, I wonder if we need the JOIN_RIGHT case here since JOIN_RIGHT
> should have been flipped around to become JOIN_LEFT.

Hmm, you're right, that's done in reduce_outer_joins which
precedes this step.

			regards, tom lane