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:
> This is the small fix, meant for 19 and master.

Sorry for having overlooked this email earlier. It's substantially
the same fix I came up with, so I credited you as co-author.
I've pushed these two fixes and marked the open item as done.

> I think a better fix would make the pathkeys right in both places,
> so the Append can keep them. I can work on that for master if you
> like.

If you're interested in working on a long-term fix, I think the path
forward ought to be to get rid of these "varno 0" Vars in favor of
using ordinary Vars that reference real RangeTblEntrys. Right now,
a Query level that represents a set-op nest only has RTE_SUBQUERY
RTEs for the leaf queries. I'm imagining inventing a new RTEKind,
say RTE_SETOP, and building one of those for each set operation
in the nest. Then the Vars representing the output columns of that
set operation could carry that RTE's index, and everything gets a
lot less magic. I'm not sure that there would be any large reduction
in total lines of code, but it'd be cleaner, and there are some things
such as tlist width estimation that would work better.

While we could move much of what's in SetOperationStmt into such
RTEs, I'd be inclined not to, because additional fields in
RangeTblEntry would just be bloat for non-SETOP RTEs. So my
druthers would be to add no new fields to RangeTblEntry, just
re-use whatever ones are there that are useful. SetOperationStmt
probably needs to gain a field for the index of the associated
RTE, though.

IIRC, there are XXX comments in prepunion.c whining about how
building the setop tlists ought to be done at parse time, so
that's something we could look into at the same time, or as a
follow-on patch.

regards, tom lane