Re: Changing SQL Inlining Behaviour (or...?)

Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com> writes:
> I've been working with Paul to create and test a patch (attached) that
> addresses Solution #2. This patch essentially modifies the inlining
> logic to compare the cost of the function with the total cost of the
> parameters. The goal as stated above, is that for these kinds of
> functions, they would be assigned relatively high cost to trigger the
> inlining case.

I played around with this a bit. I think we could make the rule simpler
and more useful by expressing it as "if the total incremental cost from
multiple evaluation of parameters exceeds the inlinable function's
nominal cost, then don't inline". The existing rule is "no individual
param expression can cost more than 10*cpu_operator_cost, if the
function uses it more than once --- never mind how many times exactly".
With the default cost (100) for a SQL language function, this'd become
"the total incremental cost can't be more than 100*cpu_operator_cost".
So that's more or less in the same ballpark, though it will make some
different decisions in cases near the boundary, and probably will choose
to inline in more cases than before. But it's simpler to compute and
more defensible as a criterion, IMO.

inlining-solution-3a.patch, attached, does it like that, keeping the
existing rule that we flat out won't inline if there are any subplans
in multiply-evaluated parameters. That seems kind of ugly, though,
so I also prepared inlining-solution-3b.patch which eliminates the
cruft around subplans in this logic by moving the cruft into
cost_qual_eval instead: it has to make an arbitrary assumption about
the cost of a SubLink.

I'm not really sure which of these I like better. The arbitrary cost
assumption in cost_qual_eval is not great, and it'd be really bad if
anyone ever tried to use costs for pre-SS_process_sublinks expressions
for something more important than the case at hand. But on the other
hand it seems nice to get rid of the arbitrary inlining choice.

Thoughts?

The other thing we need to consider is whether we need any documentation
adjustments. I believe that right now, the rules for inlining SQL
functions are not documented anywhere but the code, and maybe it's not
this patch's job to change that.

regards, tom lane