On Fri, Jan 22, 2021 at 1:16 PM Hou, Zhijie <houzj.fnst@cn.fujitsu.com> wrote:
>
>
> 4.
> + ListCell *index_expr_item = list_head(index_info->ii_Expressions);
> ...
> + index_expr = (Node *) lfirst(index_expr_item);
> + index_expr = (Node *) expression_planner((Expr *) index_expr);
>
> It seems BuildIndexInfo has already called eval_const_expressions for ii_Expressions,
> Like the flow: BuildIndexInfo--> RelationGetIndexExpressions--> eval_const_expressions
>
> So, IMO, we do not need to call expression_planner for the expr again.
>
Thanks. You are right. I debugged it, and found that BuildIndexInfo-->
RelationGetIndexExpressions executes the same expression evaluation
code as expression_planner().
So I'll remove the redundant call to expression_planner() here.
>
> And there seems another solution for this:
>
> In the patch, We only use the { ii_Expressions , ii_NumIndexAttrs , ii_IndexAttrNumbers } from the IndexInfo,
> which seems can get from "Relation-> rd_index".
>
> Based on above, May be we do not need to call BuildIndexInfo to build the IndexInfo.
> It can avoid some unnecessary cost.
> And in this solution we do not need to remove expression_planner.
>
Hmmm, when I debugged my simple test case, I found rel->rd_index was
NULL, so it seems that the call to BuildIndexInfo is needed.
(have I understood your suggestion correctly?)
Regards,
Greg Nancarrow
Fujitsu Australia
