Обсуждение: equal: don't know whether nodes of type 600 are equal

regression=> SELECT 1 AS two UNION SELECT 2;
NOTICE:  equal: don't know whether nodes of type 600 are equal
two
--- 1 2
(2 rows)

gdb:

#0  equal (a=0x179f10, b=0x1c1490) at equalfuncs.c:746
#1  0x7bf4d in remove_duplicates (list=0x167d90) at prepqual.c:563
#2  0x7b86a in qual_cleanup (qual=0x168f50) at prepqual.c:234
#3  0x7b48e in cnfify (qual=0x168790, removeAndFlag=1 '\001') at prepqual.c:76
#4  0xba22a in Except_Intersect_Rewrite (parsetree=0x179f10)              ^^^^^^^^^^^^^^^^^^^^^^^^   at
rewriteHandler.c:2842

Vadim

I chased down the complaints that were coming out of the union regress
test:

> regression=> SELECT 1 AS two UNION SELECT 2;
> NOTICE:  equal: don't know whether nodes of type 600 are equal

and found that this was a known shortcoming of the recently installed
EXCEPT/INTERSECT code:

>> -) When using UNION/EXCEPT/INTERSECT you will get:
>>    NOTICE: equal: "Don't know if nodes of type xxx are equal".
>>    I did not have  time to add comparsion support for all the needed nodes,
>>    but the default behaviour of the function equal met my requirements.
>>    I did not dare to supress this message!
>> 
>>    That's the reason why the regression test for union will fail: These
>>    messages are also included in the union.out file!

I added equality checking for T_Query and T_RangeTblEntry to
equalfuncs.c.  This is enough to persuade the regression tests
to pass again, but I suspect that some other node types still
need to be added to equal()'s repertoire.

We are definitely not out of the woods with the EXCEPT/INTERSECT changes:

test=> EXPLAIN SELECT 1 UNION SELECT 2;
ERROR:  copyObject: don't know how to copy 604

I do not think that extending copyObject to handle 604 (T_SelectStmt)
is necessarily the right fix here.  I am casting a wary eye on
the first lines of ExplainOneQuery:
/* plan the queries (XXX we've ignored rewrite!!) */plan = planner(query);

I think the real problem is that the EXCEPT/INTERSECT code is dependent
on rewrite work that is not being done in the EXPLAIN path, and that
we need to fix that underlying problem rather than patching the symptom.
Otherwise we'll likely just hit another symptom...

I'm about out of steam for today so I'll just toss that up to see if
anyone else wants to tackle it.
        regards, tom lane

Tom Lane wrote:

> I do not think that extending copyObject to handle 604 (T_SelectStmt)
> is necessarily the right fix here.  I am casting a wary eye on
> the first lines of ExplainOneQuery:
>
>    /* plan the queries (XXX we've ignored rewrite!!) */
>    plan = planner(query);
>
> I think the real problem is that the EXCEPT/INTERSECT code is dependent
> on rewrite work that is not being done in the EXPLAIN path, and that
> we need to fix that underlying problem rather than patching the symptom.
> Otherwise we'll likely just hit another symptom...

    I  added  the  call  to QueryRewrite() in ExplainQuery() some
    time ago. So the comment  in  ExplainOneQuery()  isn't  right
    (and I should have removed that).


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #

jwieck@debis.com (Jan Wieck) writes:
> Tom Lane wrote:
>> I think the real problem is that the EXCEPT/INTERSECT code is dependent
>> on rewrite work that is not being done in the EXPLAIN path, and that
>> we need to fix that underlying problem rather than patching the symptom.

>     I  added  the  call  to QueryRewrite() in ExplainQuery() some
>     time ago. So the comment  in  ExplainOneQuery()  isn't  right
>     (and I should have removed that).

ttest=> select 1 union select 2;
?column?
--------      1      2
(2 rows)

ttest=> explain select 1 union select 2;
ERROR:  copyObject: don't know how to copy 604
ttest=>

This error is coming from inside the planner (specifically union_planner).
Obviously there's *something* different about the context in which the
planner is invoked for EXPLAIN.

It looks to me like the problem is that some rewrite code got placed in
pg_parse_and_plan() in postgres.c --- there is some UNION-handling stuff
going on *after* the call to QueryRewrite(), and evidently that stuff
is not duplicated in the EXPLAIN case.  Probably the right fix is to
move all that logic inside QueryRewrite() --- but I don't want to touch
it without confirmation from someone who knows the parser/planner
better.

Some quick checks with gdb show union_planner() being invoked only once
when the query is executed for real, but recursively when using EXPLAIN
... and it's the recursive call that is throwing the error:

#0  elog (lev=-1, fmt=0x2ef20 "copyObject: don't know how to copy %d")   at elog.c:79
#1  0xae994 in copyObject (from=0x400a5028) at copyfuncs.c:1870
#2  0xae93c in copyObject (from=0x400a5250) at copyfuncs.c:1859
#3  0xc7888 in new_unsorted_tlist (targetlist=0xffffffff) at tlist.c:314
#4  0xc03a0 in union_planner (parse=0x400a5028) at planner.c:110
#5  0xc40d4 in plan_union_queries (parse=0x400a53b8) at prepunion.c:146
#6  0xc03f0 in union_planner (parse=0x400a53b8) at planner.c:131
#7  0xc0320 in planner (parse=0x400a53b8) at planner.c:80
#8  0x8dc64 in ExplainOneQuery (query=0x400a53b8, verbose=0 '\000', dest=604)   at explain.c:92
#9  0x8dc24 in ExplainQuery (query=0x400a5670, verbose=0 '\000', dest=Remote)   at explain.c:76
#10 0x1012e8 in ProcessUtility (parsetree=0x400a52d8, dest=Remote)   at utility.c:781
#11 0xfeba0 in pg_exec_query_dest (   query_string=0x7b0342b0 "explain select a from tt union select a from tt;",
dest=Remote,aclOverride=92 '\\') at postgres.c:819
 
#12 0xfe9f8 in pg_exec_query (   query_string=0xffffffff <Address 0xffffffff out of bounds>)   at postgres.c:711
#13 0xffbfc in PostgresMain (argc=316064, argv=0x51, real_argc=5,   real_argv=0x40002b10) at postgres.c:1664

I wonder whether union_planner() is even really needed anymore given the
rewrite stuff ...
        regards, tom lane

Tom Lane wrote:

> It looks to me like the problem is that some rewrite code got placed in
> pg_parse_and_plan() in postgres.c --- there is some UNION-handling stuff
> going on *after* the call to QueryRewrite(), and evidently that stuff
> is not duplicated in the EXPLAIN case.  Probably the right fix is to
> move all that logic inside QueryRewrite() --- but I don't want to touch
> it without confirmation from someone who knows the parser/planner
> better.

    It  is  the job of QueryRewrite() to puzzle/shuffle the nodes
    of parsetrees before the planner get's them.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #

> 
> This error is coming from inside the planner (specifically union_planner).
> Obviously there's *something* different about the context in which the
> planner is invoked for EXPLAIN.
> 
> It looks to me like the problem is that some rewrite code got placed in
> pg_parse_and_plan() in postgres.c --- there is some UNION-handling stuff
> going on *after* the call to QueryRewrite(), and evidently that stuff
> is not duplicated in the EXPLAIN case.  Probably the right fix is to
> move all that logic inside QueryRewrite() --- but I don't want to touch
> it without confirmation from someone who knows the parser/planner
> better.


I have been meaning to move the UNION stuff into the rewrite system
where it belongs, but haven't had time to do it.  In tcop/postgres.c,
you will see me moving through the union nodes.  That should be done at
the top of the rewrite system.  At the time, the rewrite system was so
confusing to me, I did not attempt it.  I believe that will fix the
problem.  Let me know if you need me to do it.


--  Bruce Momjian                        |  http://www.op.net/~candle maillist@candle.pha.pa.us            |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 

Bruce Momjian <maillist@candle.pha.pa.us> writes:
> I have been meaning to move the UNION stuff into the rewrite system
> where it belongs, but haven't had time to do it.  In tcop/postgres.c,
> you will see me moving through the union nodes.  That should be done at
> the top of the rewrite system.  At the time, the rewrite system was so
> confusing to me, I did not attempt it.  I believe that will fix the
> problem.  Let me know if you need me to do it.

Well, I could give it a shot, but I'm just as happy to defer to you;
I don't know anything about the rewriter and might break something.
        regards, tom lane