Re: [HACKERS] Skipping PgStat_FunctionCallUsage for many expressions

[Andres Freund]

On 2017-02-14 17:58:23 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2016-11-26 08:41:28 -0800, Andres Freund wrote:
> >> On November 26, 2016 8:06:26 AM PST, Tom Lane  wrote:
> >>> Those don't call functions, they call operators.  Yes, I know that an
> >>> operator has a function underlying it, but the user-level expectation
> >>> for track_functions is that what it counts are things that look
> >>> syntactically like function calls.  I'm not eager to add tracking
> >>> overhead for cases that there's been exactly zero field demand for.
> 
> >> But we do track for OpExprs? Otherwise I'd agree.
> 
> > Bump?
> 
> If you're going to insist on foolish consistency, I'd rather take out
> tracking in OpExpr than add it in dozens of other places.

I'm ok with being inconsistent, but I'd like to make that a conscious
choice rather it being the consequence of an oversight - and that's what
it looks like to me.

We're doing it for OpExpr, but not for a bunch of other function /
operator invocations within execQual.c (namely ExecEvalDistinct,
ExecEvalScalarArrayOp, ExecEvalRowCompare, ExecEvalMinMax,
ExecEvalNullIf), neither do we do it for *function* invocations directly
in the executor (prominently node[Window]Agg.c), but we do it for
trigger invocations.  That's, uh, a bit weird and hard to explain.

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Skipping PgStat_FunctionCallUsage for many expressions

[Tom Lane]

Andres Freund  writes:
> On 2016-11-26 08:41:28 -0800, Andres Freund wrote:
>> On November 26, 2016 8:06:26 AM PST, Tom Lane  wrote:
>>> Those don't call functions, they call operators.  Yes, I know that an
>>> operator has a function underlying it, but the user-level expectation
>>> for track_functions is that what it counts are things that look
>>> syntactically like function calls.  I'm not eager to add tracking
>>> overhead for cases that there's been exactly zero field demand for.

>> But we do track for OpExprs? Otherwise I'd agree.

> Bump?

If you're going to insist on foolish consistency, I'd rather take out
tracking in OpExpr than add it in dozens of other places.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Skipping PgStat_FunctionCallUsage for many expressions

[Andres Freund]

On 2016-11-26 08:41:28 -0800, Andres Freund wrote:
> On November 26, 2016 8:06:26 AM PST, Tom Lane  wrote:
> >Robert Haas  writes:
> >> On Fri, Nov 25, 2016 at 11:12 PM, Andres Freund 
> >wrote:
> >>> while working on my faster expression evaluation stuff I noticed
> >that a
> >>> lot of expression types that call functions don't call the necessary
> >>> functions to make track_functions work.
> >>>
> >>> ExecEvalFunc/ExecEvalOper (via ExecMakeFunctionResultNoSets) call
> >>> pgstat_init_function_usage/pgstat_end_function_usage, but others
> >like
> >>> ExecEvalRowCompare, ExecEvalMinMax, ExecEvalNullIf,
> >ExecEvalDistinct,
> >>> ExecEvalScalarArrayOp (and indirectly ExecEvalArrayCoerceExpr)
> >don't.
> >>>
> >>> Similarly InvokeFunctionExecuteHook isn't used very thoroughly.
> >>>
> >>> Are these worth fixing? I suspect yes. If so, do we want to
> >backpatch?
> >
> >Those don't call functions, they call operators.  Yes, I know that an
> >operator has a function underlying it, but the user-level expectation
> >for
> >track_functions is that what it counts are things that look
> >syntactically
> >like function calls.  I'm not eager to add tracking overhead for cases
> >that there's been exactly zero field demand for.
>
> But we do track for OpExprs? Otherwise I'd agree.

Bump?

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Skipping PgStat_FunctionCallUsage for many expressions

[Andres Freund]

On November 26, 2016 8:06:26 AM PST, Tom Lane  wrote:
>Robert Haas  writes:
>> On Fri, Nov 25, 2016 at 11:12 PM, Andres Freund 
>wrote:
>>> while working on my faster expression evaluation stuff I noticed
>that a
>>> lot of expression types that call functions don't call the necessary
>>> functions to make track_functions work.
>>> 
>>> ExecEvalFunc/ExecEvalOper (via ExecMakeFunctionResultNoSets) call
>>> pgstat_init_function_usage/pgstat_end_function_usage, but others
>like
>>> ExecEvalRowCompare, ExecEvalMinMax, ExecEvalNullIf,
>ExecEvalDistinct,
>>> ExecEvalScalarArrayOp (and indirectly ExecEvalArrayCoerceExpr)
>don't.
>>> 
>>> Similarly InvokeFunctionExecuteHook isn't used very thoroughly.
>>> 
>>> Are these worth fixing? I suspect yes. If so, do we want to
>backpatch?
>
>Those don't call functions, they call operators.  Yes, I know that an
>operator has a function underlying it, but the user-level expectation
>for
>track_functions is that what it counts are things that look
>syntactically
>like function calls.  I'm not eager to add tracking overhead for cases
>that there's been exactly zero field demand for.

But we do track for OpExprs? Otherwise I'd agree.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Skipping PgStat_FunctionCallUsage for many expressions

[Tom Lane]

Robert Haas  writes:
> On Fri, Nov 25, 2016 at 11:12 PM, Andres Freund  wrote:
>> while working on my faster expression evaluation stuff I noticed that a
>> lot of expression types that call functions don't call the necessary
>> functions to make track_functions work.
>> 
>> ExecEvalFunc/ExecEvalOper (via ExecMakeFunctionResultNoSets) call
>> pgstat_init_function_usage/pgstat_end_function_usage, but others like
>> ExecEvalRowCompare, ExecEvalMinMax, ExecEvalNullIf, ExecEvalDistinct,
>> ExecEvalScalarArrayOp (and indirectly ExecEvalArrayCoerceExpr) don't.
>> 
>> Similarly InvokeFunctionExecuteHook isn't used very thoroughly.
>> 
>> Are these worth fixing? I suspect yes. If so, do we want to backpatch?

Those don't call functions, they call operators.  Yes, I know that an
operator has a function underlying it, but the user-level expectation for
track_functions is that what it counts are things that look syntactically
like function calls.  I'm not eager to add tracking overhead for cases
that there's been exactly zero field demand for.

> If it doesn't torpedo performance, I assume we should fix and back-patch.

It's certainly not going to help.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Skipping PgStat_FunctionCallUsage for many expressions

[Robert Haas]

On Fri, Nov 25, 2016 at 11:12 PM, Andres Freund  wrote:
> while working on my faster expression evaluation stuff I noticed that a
> lot of expression types that call functions don't call the necessary
> functions to make track_functions work.
>
> ExecEvalFunc/ExecEvalOper (via ExecMakeFunctionResultNoSets) call
> pgstat_init_function_usage/pgstat_end_function_usage, but others like
> ExecEvalRowCompare, ExecEvalMinMax, ExecEvalNullIf, ExecEvalDistinct,
> ExecEvalScalarArrayOp (and indirectly ExecEvalArrayCoerceExpr) don't.
>
> Similarly InvokeFunctionExecuteHook isn't used very thoroughly.
>
> Are these worth fixing? I suspect yes. If so, do we want to backpatch?

If it doesn't torpedo performance, I assume we should fix and back-patch.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers