Re: FIX : teach expression walker about RestrictInfo

On Fri, Mar 18, 2016 at 3:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Apr 29, 2015 at 12:33 PM, Tomas Vondra
>> <tomas.vondra@2ndquadrant.com> wrote:
>>> On 04/29/15 18:26, Tom Lane wrote:
>>>> But there are basic reasons why expression_tree_walker should not try
>>>> to deal with RestrictInfos; the most obvious one being that it's not
>>>> clear whether it should descend into both the basic and OR-clause
>>>> subtrees. Also, if a node has expression_tree_walker support then it
>>>> should logically have expression_tree_mutator support as well, but
>>>> that's got multiple issues. RestrictInfos are not supposed to be
>>>> copied once created; and the mutator couldn't detect whether their
>>>> derived fields are still valid.
>
>>> OK, I do understand that. So what about pull_varnos_walker and
>>> pull_varattnos_walker - what about teaching them about RestrictInfos?
>
>> This patch has become part 1 of many under the "multivariate
>> statistics vNNN" family of threads, but I guess it would be helpful if
>> you could opine on the reasonable-ness of this approach.  I don't want
>> to commit anything over your objections, but this kind of tailed off
>> without a conclusion.
>
> I'm up to my ears in psql at the moment, but hope to get to the
> multivariate stats patch later, maybe next week.

Oh, cool.

> In the meantime,
> I remain of the opinion that RestrictInfo is not an expression node
> and wanting expression_tree_walker to handle it is wrong.  I'm
> suspicious about pull_varnos; it might be all right given that we
> can assume the same Vars are in both subtrees, but I still don't
> really see why that one function has suddenly grown this need when
> others have not.

I haven't studied the patch series in enough detail to have an
educated opinion on that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company