Re: Handling RestrictInfo in expression_tree_walker - Mailing list pgsql-hackers

From Konstantin Knizhnik
Subject Re: Handling RestrictInfo in expression_tree_walker
Date
Msg-id 8cab2118-e742-f416-0da7-6ffcfe6e2293@postgrespro.ru
Whole thread Raw
In response to Re: Handling RestrictInfo in expression_tree_walker  (Amit Langote <amitlangote09@gmail.com>)
Responses Re: Handling RestrictInfo in expression_tree_walker
Re: Handling RestrictInfo in expression_tree_walker
List pgsql-hackers

On 07.08.2019 10:42, Amit Langote wrote:
> Hi Konstantin,
>
> On Wed, Aug 7, 2019 at 4:24 PM Konstantin Knizhnik
> <k.knizhnik@postgrespro.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




pgsql-hackers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: POC: Cleaning up orphaned files using undo logs
Next
From: Masahiko Sawada
Date:
Subject: Re: [Proposal] Table-level Transparent Data Encryption (TDE) and KeyManagement Service (KMS)