"Anton A. Melnikov" <[email protected]> writes:
> With help of massif tool i found repeated allocations originating from:
>      ExecInitGenerated
>        → build_column_default
>          → stringToNode
> This indicates that generated expressions are reparsed multiple times,
> once per row to be updated instead of being reused.

Hm.

> I would like to propose a fix that add a caching of the the parsed
> expression trees (Node *) in ResultRelInfo, so that build_column_default()
> and stringToNode() are executed at most once per attribute per query.

That seems pretty brute-force.  Both ExecInitGenerated and its callers
are still of the opinion that it's only done once per query; what is
broken about that?

[ pokes at it... ]  It looks like the problem in this example is
that the trivial-case path

        if (ri_NumGeneratedNeeded == 0)
        {
                /* didn't need it after all */
                pfree(ri_GeneratedExprs);
                ri_GeneratedExprs = NULL;
        }

results in storing NULL into ri_GeneratedExprsU, so that the
next time through ExecComputeStoredGenerated, we still think
we need to do ExecInitGenerated:

                if (resultRelInfo->ri_GeneratedExprsU == NULL)
                        ExecInitGenerated(resultRelInfo, estate, cmdtype);

So I think the correct fix is that there needs to be a separate
boolean tracking whether this work has been done, akin to
ExecGetExtraUpdatedCols's use of ri_extraUpdatedCols_valid.

                        regards, tom lane


Reply via email to