On Fri, Jan 22, 2021 at 1:16 PM Hou, Zhijie <[email protected]> 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
