Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"

shihao zhong <zhong950419(at)gmail(dot)com> writes:
> Hi Tom,
>> There are other calls to create_append_path in prepunion.c, and I
>> think they may all need to do likewise, but I didn't analyze them.

> The EXCEPT ALL one needs it too. With only your patch this still
> fails:
> set enable_hashagg = off;
> (select a from d where a = 1 intersect select a from d where a = 1)
> except all select a from d where false;

Ah. I'd been trying to make a test case for that one, but I didn't
realize that two levels of setop are required. Something like this
doesn't fail:

explain
select * from
((select * from int8_tbl i81 order by q1)
except all (select * from int8_tbl i81b where false)) ss1,
((select * from int8_tbl i82 order by q1)
except all (select * from int8_tbl i82b where false)) ss2
where ss1.q1 = ss2.q1
;

However, digging into the guts of that doesn't leave a warm feeling
either. Unpatched, we end up with the same situation where a
single-child AppendPath has a tlist containing varno-0 Vars and a
pathkey, and create_append_plan tries to compute sort column info from
that. The reason it fails to fail is that *the equivalence classes
contain varno-0 Vars too*. I've not entirely figured out why this
is different from the original test case --- well, okay, UNION ALL
at the top level is different because it doesn't make any pathkeys,
but if you change that to UNION the test case still fails on unpatched
code, and in that case the pathkey-slinging sure looks the same.

Regardless of the detailed reason for that, having varno-0 Vars in
equivalence classes scares the dickens out of me. Each setop node
is going to have its own varno-0 Vars, and if they match on type then
the equivalence class machinery can't tell them apart, so it sure
seems like we are at risk of drawing false conclusions about whether
different setop outputs are sorted alike. In the above example I was
trying to break it by having it falsely deduce that the outer WHERE
clause could be thrown away or reduced to an IS NOT NULL test.
I failed, which turns out to be because the questionable eclasses are
not at top level but within the two subroots associated with the two
setop nests. So I think the potential bad effects are limited to
maybe mistakenly planning a single setop nest, and so far we've
escaped issues mainly because we don't do that much optimization of
non-UNION-ALL nests. But it's really past time to get rid of the
varno-0 representation. For now, one reason I like forcing these
paths' pathkeys to nil is that it limits the amount of damage that
could be done by false equivalence-class reasoning. In particular,
I wonder whether v19/HEAD are at risk of such bugs in cases that
couldn't occur before we started eliding dummy child setops.

regards, tom lane