> On Jan 21, 2019, at 3:17 PM, Andres Freund <and...@anarazel.de> wrote: > > Hi, > > On 2019-01-19 20:53:55 -0500, Tom Lane wrote: >> I wrote: >>> Paul Ramsey <pram...@cleverelephant.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
