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

> On Jan 21, 2019, at 3:17 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2019-01-19 20:53:55 -0500, Tom Lane wrote:
>> I wrote:
>>> Paul Ramsey <pramsey(at)cleverelephant(dot)ca> writes:
>>>> Is there a rule of thumb we can use in costing our wrappers to ensure that they always inline?
>>
>>> Not really, which is a weak spot of this whole approach.
>>
>> BTW, it suddenly strikes me that at least for the specific cases you've
>> shown in this thread, worrying about forcing inlining despite multiple
>> parameter occurrences is solving the wrong problem. As I understand it,
>> what you're doing is basically expanding some_operation(x,y) into
>>
>> x indexable_operator y AND exact_condition(x,y)
>>
>> where exact_condition(x,y) is semantically identical to
>> some_operation(x,y), and the point of the maneuver is to inject
>> an indexable operator that implements some lossy form of the
>> exact_condition. But this is not an ideal solution: aside
>> from the possible cost of evaluating x and y twice, adding
>> the indexable_operator is just a dead loss if you don't end up
>> with an indexscan on x.
>>
>> So ISTM what we *really* want is that the wrapper function is
>> just an alias for exact_condition() --- which eliminates the
>> multiple-evaluation hazards and hence the risk of not inlining ---
>> and then teach the planner how to extract the indexable_operator
>> when, and only when, it's actually useful for an indexscan.
>>
>> The machinery for that sort of thing already exists; it's what
>> indxpath.c calls a "special index operator". But right now it's
>> only applied to some built-in operators such as LIKE. I've had
>> a personal TODO item for a very long time to make that ability
>> accessible to extensions, but it never rose to the top of the
>> queue. Maybe we should be trying to make that happen, instead
>> of messing around with dubious changes to the inlining rules.
>>
>> Does this line of thought seem like it would fix your problem,
>> or are there places where the inlining rules are causing you
>> issues but the use-case doesn't look like "add a lossy indexable
>> operator"?
>
> Paul, is what Tom calls indexable_operator here just useful for
> indexability, or *also* useful to reduce the frequency of invoking the
> exact_condition(), which presumably will frequently be much more
> expensive to evaluate? With the current scheme, as long as it works,
> the indexable fastpath filters out rows quickly for both sequential and
> index scans, but I'm not sure that'd work in the transformation scheme
> Tom proposes. Sure, the exact_condition could always do the cheaper
> pre-check, but that'd be wasted effort in the index scan case, where
> that'd presumably already be ruled out.
>
> Actually an issue, or not?

As a practical matter, most of the exact-test functions have a preamble that checks the bbox, so in the seqscan case having the operator along for the ride isn’t any advantage. In any event, if we do have exact tests w/o a lossy preamble, we could add that for v12, as this renovation won’t be a small one if we go this direction.

My only concern, as a plebian, is that what he have, hacky or otherwise, works, and the quick’n’dirty solution also works, and the new hotness seems quite complex, relatively speaking. I guess the new hotness also gets us the ability to do selectivity at a function level, which is also something we are interested in the future. so there’s that, but … I’d just be happy to solve the hacky inline problem :)

P

> Greetings,
>
> Andres Freund