Re: [PATCHES] WIP: Hierarchical Queries - stage 1

[Mark Cave-Ayland <mark(dot)cave-ayland(at)ilande(dot)co(dot)uk>]

Hi Tom,

Thanks for your initial thoughts on this.


On Wed, 2006-09-20 at 20:13 -0400, Tom Lane wrote:

(cut)

> You really can't get away with having the identical representation for
> CTEs and ordinary sub-selects in the range table.  For instance, it
> looks like your patch will think that
> 
> 	select ... from (select ...) as x, x, ...
> 
> is legal when it certainly is not.  I think you need either a new
> RTEKind or an additional flag in the RTE to show that it's a CTE rather
> than a plain subselect.  I'm not entirely sure that you even want the
> CTEs in the rangetable at all --- that still needs some thought.

For semantic reasons, I can see why you are questioning whether or not
the CTE should be contained within the rangetable - there is an implicit
hint that RTEs reflect entries within the FROM clause, but then I also
see that the rewriter adds RTEs when substituting view definitions into
queries. The comments in parsenodes.h also suggest that an RTE is a
namespace/data source reference for a named entity within the query.

The main problem I can see with keeping the CTEs outside the rangetable
is that according to the source, jointree nodes must currently have
RANGETBLREF nodes as leaf nodes; as I understand it, your suggestion of
maintaining the CTEs separately would involve something along the lines
of keeping a separate CTETable and creating some form of CTETBLREF node
that could be referenced within the jointree. While arguably it may be
semantically neater, it appears to involve quite a bit of extra work...
could you explain in more detail as to why you feel that CTEs should
remain outside the rangetable?

> This comes back to the question of whether the CTE per se should be an
> RTE at all.  Maybe only the reference to it should be an RTE.  The
> behavior when seeing a plain RangeVar in FROM would be to first search
> the side list of valid CTEs, and only on failure go looking for a real
> table.

This is effectively what the patch does, albeit not particularly
elegantly. I'll spend some time on making those changes a bit more
refined.


Kind regards,

Mark.