Re: Handling RestrictInfo in expression_tree_walker

On 07.08.2019 10:42, Amit Langote wrote:
> Hi Konstantin,
>
> On Wed, Aug 7, 2019 at 4:24 PM Konstantin Knizhnik
> <k(dot)knizhnik(at)postgrespro(dot)ru> wrote:
>> Hi hackers,
>>
>> I wonder if there is some particular reason for not handling
>> T_RestrictInfo node tag in expression_tree_walker?
>> There are many data structure in Postgres which contains lists of
>> RestrictInfo or expression with RestrictInfo as parameter (for example
>> orclause in RestrictInfo).
>> To handle such cases now it is needed to write code performing list
>> iteration and calling expression_tree_walker for each list element and
>> handling RrestrictInfo in callback function:
>>
>> static bool
>> change_varno_walker(Node *node, ChangeVarnoContext *context)
>> {
>> if (node == NULL)
>> return false;
>>
>> if (IsA(node, Var) && ((Var *) node)->varno == context->oldRelid)
>> {
>> ((Var *) node)->varno = context->newRelid;
>> ((Var *) node)->varnoold = context->newRelid;
>> return false;
>> }
>> if (IsA(node, RestrictInfo))
>> {
>> change_rinfo((RestrictInfo*)node, context->oldRelid,
>> context->newRelid);
>> return false;
>> }
>> return expression_tree_walker(node, change_varno_walker, context);
>> }
>>
>> Are there any complaints against handling RestrictInfo in
>> expression_tree_walker?
> As I understand it, RestrictInfo is not something that appears in
> query trees or plan trees, but only in the planner data structures as
> means of caching some information about the clauses that they wrap. I
> see this comment describing what expression_tree_walker() is supposed
> to handle:
>
> * The node types handled by expression_tree_walker include all those
> * normally found in target lists and qualifier clauses during the planning
> * stage.
>
> You may also want to read this discussion:
>
> https://www.postgresql.org/message-id/553FC9BC.5060402%402ndquadrant.com
>
> Thanks,
> Amit
Thank you very much for response and pointing me to this thread.
Unfortunately I do not understand from this thread how the problem was
solved with pullvars - right now  pull_varnos_walker and
pull_varattnos_walker
are not handling RestrictInfo.

Also I do not completely understand the argument "RestrictInfo is not a
general expression node and support for it has
been deliberately omitted from expression_tree_walker()". If there is
BoolOp expression which contains RestrictInfo expression as it
arguments, then either this expression is not
correct, either RestrictInfo should be considered as "expression node".

Frankly speaking I do not see some good reasons for not handling
RestrictInfo in expression_tree_worker. It can really simplify writing
of mutators/walkers.
And I do not think that reporting error instead of handling this tag
adds some extra safety or error protection.

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company