Обсуждение: BUG #17826: An assert failed in /src/backend/optimizer/util/var.c
The following bug has been logged on the website: Bug reference: 17826 Logged by: Anban Company Email address: xinwen@stu.scu.edu.cn PostgreSQL version: 15.2 Operating system: Ubuntu 20.04 Description: When executing the following query: WITH table1 AS ( SELECT 1 ) SELECT 1 FROM ( SELECT RANK ( ) OVER ( ) column0 FROM table1 JOIN table1 AS alias0 ON TRUE ) AS alias1 WHERE ( CASE WHEN ( 1 IN ( SELECT 1 ) ) THEN 1 END = column0 ) ; 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 0x00007f68a29b6859 in __GI_abort () at abort.c:79 #2 0x0000555d7a2db598 in ExceptionalCondition (conditionName=conditionName@entry=0x555d7a44c828 "!IsA(node, SubPlan)", errorType=errorType@entry=0x555d7a3394a0 "FailedAssertion", fileName=fileName@entry=0x555d7a4504b8 "/home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/util/var.c", lineNumber=lineNumber@entry=872) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/utils/error/assert.c:69 #3 0x0000555d7a105c41 in flatten_join_alias_vars_mutator (node=0x555d7aa0f518, context=0x7fff50b0c980) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/util/var.c:872 #4 0x0000555d7a08cba9 in expression_tree_mutator (node=0x555d7aa07eb0, mutator=0x555d7a1057a0 <flatten_join_alias_vars_mutator>, context=0x7fff50b0c980) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/nodes/nodeFuncs.c:3320 #5 0x0000555d7a08cff6 in expression_tree_mutator (node=0x555d7aa0f888, mutator=0x555d7a1057a0 <flatten_join_alias_vars_mutator>, context=0x7fff50b0c980) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/nodes/nodeFuncs.c:3165 #6 0x0000555d7a08cef9 in expression_tree_mutator (node=0x555d7aa07e58, mutator=0x555d7a1057a0 <flatten_join_alias_vars_mutator>, context=0x7fff50b0c980) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/nodes/nodeFuncs.c:3137 #7 0x0000555d7a08cff6 in expression_tree_mutator (node=0x555d7aa11ba0, mutator=0x555d7a1057a0 <flatten_join_alias_vars_mutator>, context=0x7fff50b0c980) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/nodes/nodeFuncs.c:3165 #8 0x0000555d7a08ccab in expression_tree_mutator (node=0x555d7aa11400, mutator=0x555d7a1057a0 <flatten_join_alias_vars_mutator>, context=0x7fff50b0c980) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/nodes/nodeFuncs.c:2780 #9 0x0000555d7a08cff6 in expression_tree_mutator (node=0x555d7aa11bf8, mutator=mutator@entry=0x555d7a1057a0 <flatten_join_alias_vars_mutator>, context=context@entry=0x7fff50b0c980) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/nodes/nodeFuncs.c:3165 #10 0x0000555d7a105821 in flatten_join_alias_vars_mutator (node=<optimized out>, context=context@entry=0x7fff50b0c980) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/util/var.c:878 #11 0x0000555d7a10602e in flatten_join_alias_vars (query=<optimized out>, node=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/util/var.c:744 #12 0x0000555d7a0d4b79 in preprocess_expression (root=root@entry=0x555d7aa11ca8, expr=<optimized out>, kind=kind@entry=1) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/plan/planner.c:1105 #13 0x0000555d7a0dcb8c in subquery_planner (glob=<optimized out>, parse=parse@entry=0x555d7aa10058, parent_root=parent_root@entry=0x555d7aa05f78, hasRecursion=hasRecursion@entry=false, tuple_fraction=0) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/plan/planner.c:829 #14 0x0000555d7a0ac11e in set_subquery_pathlist (rte=<optimized out>, rti=<optimized out>, rel=<optimized out>, root=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/path/allpaths.c:2601 #15 set_rel_size (root=<optimized out>, rel=<optimized out>, rti=<optimized out>, rte=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/path/allpaths.c:425 #16 0x0000555d7a0ae980 in set_base_rel_sizes (root=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/path/allpaths.c:326 #17 make_one_rel (root=root@entry=0x555d7aa05f78, joinlist=joinlist@entry=0x555d7aa0fe18) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/path/allpaths.c:188 #18 0x0000555d7a0d4976 in query_planner (root=root@entry=0x555d7aa05f78, qp_callback=qp_callback@entry=0x555d7a0d5990 <standard_qp_callback>, qp_extra=qp_extra@entry=0x7fff50b0ccb0) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/plan/planmain.c:276 #19 0x0000555d7a0da4f9 in grouping_planner (root=<optimized out>, tuple_fraction=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/plan/planner.c:1470 #20 0x0000555d7a0dd145 in subquery_planner (glob=glob@entry=0x555d7aa047d8, parse=parse@entry=0x555d7aa02e98, parent_root=parent_root@entry=0x0, hasRecursion=hasRecursion@entry=false, tuple_fraction=tuple_fraction@entry=0) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/plan/planner.c:1047 #21 0x0000555d7a0dd5a3 in standard_planner (parse=0x555d7aa02e98, query_string=<optimized out>, cursorOptions=2048, boundParams=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/optimizer/plan/planner.c:406 #22 0x0000555d7a1af74c in pg_plan_query (querytree=0x555d7aa02e98, query_string=0x555d7a93c010 "WITH table1 AS ( SELECT 1 ) SELECT 1 FROM ( SELECT RANK ( ) OVER ( ) column46 FROM table1 JOIN table1 AS alias0 ON TRUE ) AS alias1 WHERE ( CASE WHEN ( 1 IN ( SELECT 1 ) ) THEN 1 END = column46 ) ;", cursorOptions=2048, boundParams=0x0) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/tcop/postgres.c:883 #23 0x0000555d7a1af841 in pg_plan_queries (querytrees=0x555d7aa05f20, query_string=query_string@entry=0x555d7a93c010 "WITH table1 AS ( SELECT 1 ) SELECT 1 FROM ( SELECT RANK ( ) OVER ( ) column46 FROM table1 JOIN table1 AS alias0 ON TRUE ) AS alias1 WHERE ( CASE WHEN ( 1 IN ( SELECT 1 ) ) THEN 1 END = column46 ) ;", cursorOptions=cursorOptions@entry=2048, boundParams=boundParams@entry=0x0) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/tcop/postgres.c:975 #24 0x0000555d7a1afc4f in exec_simple_query ( query_string=0x555d7a93c010 "WITH table1 AS ( SELECT 1 ) SELECT 1 FROM ( SELECT RANK ( ) OVER ( ) column46 FROM table1 JOIN table1 AS alias0 ON TRUE ) AS alias1 WHERE ( CASE WHEN ( 1 IN ( SELECT 1 ) ) THEN 1 END = column46 ) ;") at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/tcop/postgres.c:1169 #25 0x0000555d7a1b17fc in PostgresMain (dbname=<optimized out>, username=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/tcop/postgres.c:4593 #26 0x0000555d7a11e5aa in BackendRun (port=<optimized out>, port=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/postmaster/postmaster.c:4511 #27 BackendStartup (port=<optimized out>) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/postmaster/postmaster.c:4239 #28 ServerLoop () at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/postmaster/postmaster.c:1806 #29 0x0000555d7a11f71b in PostmasterMain (argc=<optimized out>, argv=0x555d7a936310) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/postmaster/postmaster.c:1478 #30 0x0000555d79e49510 in main (argc=3, argv=0x555d7a936310) at /home/postgres/postgresql-15.2/original_bin-15.2/../src/backend/main/main.c:202
PG Bug reporting form <noreply@postgresql.org> writes:
> When executing the following query:
> WITH table1 AS ( SELECT 1 ) SELECT 1 FROM ( SELECT RANK ( ) OVER ( ) column0
> FROM table1 JOIN table1 AS alias0 ON TRUE ) AS alias1 WHERE ( CASE WHEN ( 1
> IN ( SELECT 1 ) ) THEN 1 END = column0 ) ;
> I get a failed assertion with the following stacktrace:
The assertion is complaining about a SubPlan that has gotten pushed down
into the subquery, where it should not have been. Tracing through it,
the blame appears to fall on commit 9d9c02ccd, which added this rather
astonishing bit of coding:
if (!rinfo->pseudoconstant &&
qual_is_pushdown_safe(subquery, rti, rinfo, &safetyInfo))
{
/* Push it down */
subquery_push_qual(subquery, rte, rti, clause);
}
else
{
+ /*
+ * Since we can't push the qual down into the subquery, check
+ * if it happens to reference a window function. If so then
+ * it might be useful to use for the WindowAgg's runCondition.
+ */
+ if (!subquery->hasWindowFuncs ||
+ check_and_push_window_quals(subquery, rte, rti, clause,
+ &run_cond_attrs))
In other words, after qual_is_pushdown_safe has rejected a qual as being
unsafe to push down, check_and_push_window_quals merrily does it anyway,
apparently on the theory that window functions are exempt from all rules.
How is this even a little bit sane?
We could fix the given problem by adding a duplicative contain_subplans
check in check_and_push_window_quals, but first I want to see an argument
why it's okay to ignore any of the other things qual_is_pushdown_safe is
checking for, because I think this is dangerously broken on every single
count. My druthers would be to remove check_and_push_window_quals
altogether.
regards, tom lane
On Fri, 10 Mar 2023 at 05:09, Tom Lane <tgl@sss.pgh.pa.us> wrote: > In other words, after qual_is_pushdown_safe has rejected a qual as being > unsafe to push down, check_and_push_window_quals merrily does it anyway, > apparently on the theory that window functions are exempt from all rules. > How is this even a little bit sane? I don't think it is. I've not looked in detail yet, but I think we might be able to do something in check_output_expressions() so that we track the targetIsInAllPartitionLists() separately. Maybe unsafeColumns[] needs to become an enum type that has UNSAFE, SAFE, WINDOW_RUNCONDITION_ONLY and then change things so we never make a runCondition when the qual is UNSAFE. Anyway, I'll give it more thought and aim to come up with a patch early next week. David
David Rowley <dgrowleyml@gmail.com> writes:
> On Fri, 10 Mar 2023 at 05:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> How is this even a little bit sane?
> I don't think it is. I've not looked in detail yet, but I think we
> might be able to do something in check_output_expressions() so that we
> track the targetIsInAllPartitionLists() separately. Maybe
> unsafeColumns[] needs to become an enum type that has UNSAFE, SAFE,
> WINDOW_RUNCONDITION_ONLY and then change things so we never make a
> runCondition when the qual is UNSAFE.
> Anyway, I'll give it more thought and aim to come up with a patch
> early next week.
Why can't we simply *remove* all this logic from subquery pushdown?
I would expect that what happens in useful cases is that we push down
the (safe) qual and then during planning of the subquery we see a WHERE
clause matching the window run condition and work with that. So this
all seems redundant, quite aside from being faulty.
regards, tom lane
On Sun, 12 Mar 2023 at 06:24, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Why can't we simply *remove* all this logic from subquery pushdown? > > I would expect that what happens in useful cases is that we push down > the (safe) qual and then during planning of the subquery we see a WHERE > clause matching the window run condition and work with that. So this > all seems redundant, quite aside from being faulty. hmm, sorry, I can't quite make sense of that. It is only possible to either leave such quals in the outer query in the WHERE clause or make them run conditions of some WindowClause in the subquery. They can never be legally in the WHERE clause of the subquery as window functions cannot be evaluated in the WHERE clause. So I'm not really sure what the point of temporarily putting it in the subqueries WHERE clause would be if we just later unconditionally move it to become a WindowClause run condition. If we're pushing it down, then we need to validate that the qual definitely can become a run condition qual, why not just make it that when it passes the validation? David
David Rowley <dgrowleyml@gmail.com> writes:
> It is only possible to either leave such quals in the outer query in
> the WHERE clause or make them run conditions of some WindowClause in
> the subquery. They can never be legally in the WHERE clause of the
> subquery as window functions cannot be evaluated in the WHERE clause.
Ah, got it. So basically, the quals that we want are excluded by
check_output_expressions' point 4 (since they reference window
function output columns rather than partitioning columns). I think
we need them to satisfy every other property that's checked in this
code, though. Now I agree that we need to refactor a bit -- we
don't want to have to re-check all of these conditions.
regards, tom lane
On Mon, 13 Mar 2023 at 10:43, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > David Rowley <dgrowleyml@gmail.com> writes: > > It is only possible to either leave such quals in the outer query in > > the WHERE clause or make them run conditions of some WindowClause in > > the subquery. They can never be legally in the WHERE clause of the > > subquery as window functions cannot be evaluated in the WHERE clause. > > Ah, got it. So basically, the quals that we want are excluded by > check_output_expressions' point 4 (since they reference window > function output columns rather than partitioning columns). I think > we need them to satisfy every other property that's checked in this > code, though. Now I agree that we need to refactor a bit -- we > don't want to have to re-check all of these conditions. OK, patch attached. I've coded it so the only safety hazard that we can ignore for run conditions is the column is in all WindowClause PARTITION BYs. I believe the DISTINCT ON column not in the sort list is still a hazard. The attached also fixes the issue with making a run condition when the window func contains a volatile func, e.g: explain select * from (select oid,count(random()) over (order by oid) c from pg_class) c where c < 10; I think this is ok to back patch to v15 as the pushdown_safety_info struct is static. I think it'd be ok to widen that type to int instead of unsigned char, I just didn't. I probably need to do a bit more indenting work. I'll look again tomorrow. David
Вложения
David Rowley <dgrowleyml@gmail.com> writes:
> On Mon, 13 Mar 2023 at 10:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Ah, got it. So basically, the quals that we want are excluded by
>> check_output_expressions' point 4 (since they reference window
>> function output columns rather than partitioning columns). I think
>> we need them to satisfy every other property that's checked in this
>> code, though. Now I agree that we need to refactor a bit -- we
>> don't want to have to re-check all of these conditions.
> OK, patch attached.
This looks good to me. As a matter of style, I'd capitalize the
enum constant names, but I think it's OK otherwise.
> I probably need to do a bit more indenting work. I'll look again tomorrow.
Yeah, this looks like pgindent might not like it as-is.
regards, tom lane
On Fri, 17 Mar 2023 at 07:59, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > David Rowley <dgrowleyml@gmail.com> writes: > > OK, patch attached. > > This looks good to me. As a matter of style, I'd capitalize the > enum constant names, but I think it's OK otherwise. Thanks. I had been doing a bit more work on it and it looks very slightly different from when you saw it. Agreed with the capitals in the enum values. I also found had to disallow subqueries in the WindowFunc. That required a subplan check inside find_window_run_conditions(). I kinda also wanted to do the attached so that the logic to push or not to push was more centralised. I just couldn't figure out exactly why we don't push down pseudoconstant quals, therefore, was unable to document that sufficiently in the header comment in qual_is_pushdown_safe(). I thought we might as well talk about that while on the topic as what I've just committed does not really take any measures to explain it. The attached patch is graded for the assistance of discussion only. David
Вложения
David Rowley <dgrowleyml@gmail.com> writes:
> I kinda also wanted to do the attached so that the logic to push or
> not to push was more centralised. I just couldn't figure out exactly
> why we don't push down pseudoconstant quals, therefore, was unable to
> document that sufficiently in the header comment in
> qual_is_pushdown_safe().
A pseudoconstant qual turns into a filtering Result node with a one-time
condition that controls whether it runs the child plan at all. You want
to put those as high in the plan tree as possible, so that they cut off
the maximum possible amount of work when the one-time condition is false.
So pushing one down into a subplan would be downright counterproductive.
regards, tom lane