Hi,

On 04/29/15 05:55, Tom Lane wrote:
Tomas Vondra <tomas.von...@2ndquadrant.com> writes:
On 04/28/15 21:50, Tom Lane wrote:
RestrictInfo is not a general expression node and support for it has
been deliberately omitted from expression_tree_walker().  So I think
what you are proposing is a bad idea and probably a band-aid for some
other bad idea.

That's not what I said, though. I said that calling pull_varnos() causes
the issue - apparently that does not happen in master, but I ran into
that when hacking on a patch.

pull_varnos is not, and should not be, applied to a RestrictInfo; for one
thing, it'd be redundant with work that was already done when creating the
RestrictInfo (cf make_restrictinfo_internal).  You've not shown the
context of your problem very fully, but in general most code in the planner
should be well aware of whether it is working with a RestrictInfo (or list
of same) or a bare expression.  I don't believe that it's a very good idea
to obscure that distinction.

OK, let me explain the context a bit more. When working on the multivariate statistics patch, I need to choose which stats to use for estimating the clauses. I do that in clauselist_selectivity(), although there might be other places where to do that.

Selecting the stats that best match the clauses is based on how well they cover the vars in the clauses (and some other criteria, that is mostly irrelevant here). So at some point I do call functions like pull_varnos() and pull_varattnos() to get this information.

I do handle RestrictInfo nodes explicitly - for those nodes I can do get the info from the node itself. But this does not work for the condition I posted, because that contains nested RestrictInfo nodes. So I do call pull_varnos() on a BoolExpr node, but in reality the node tree representing the clauses

    WHERE (a >= 10 AND a <= 20) OR (b >= 20 AND b <= 40)

looks like this:

    BoolExpr [OR_EXPR]
        BoolExpr [AND_EXPR]
            RestrictInfo
                OpExpr [Var >= Const]
            RestrictInfo
                OpExpr [Var <= Const]
        BoolExpr [AND_EXPR]
            RestrictInfo
                OpExpr [Var >= Const]
            RestrictInfo
                OpExpr [Var <= Const]

so the pull_varnos() fails because it runs into RestrictInfo node while walking the tree.

So I see three possibilities:

1) pull_varnos/pull_varattnos (and such functions, using the
   expression walker internally) are not supposed to be called unless
   you are sure there are no RestrictInfo nodes in the tree, but this
   seems really awkward

2) expression walker is supposed to know about RestrictInfo nodes (as I
   did in my patch), but you say this is not the case

3) pull_varnos_walker / pull_varattnos_walker are supposed to handle
   RestrictInfo nodes explicitly (either by using the cached information
   or by using RestrictInfo->clause in the next step)


regards

--
Tomas Vondra                   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to