Skipping PgStat_FunctionCallUsage for many expressions

On Fri, Nov 25, 2016 at 11:12 PM, Andres Freund <andres@anarazel.de> 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

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Nov 25, 2016 at 11:12 PM, Andres Freund <andres@anarazel.de> 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

On November 26, 2016 8:06:26 AM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Nov 25, 2016 at 11:12 PM, Andres Freund <andres@anarazel.de>

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

On 2016-11-26 08:41:28 -0800, Andres Freund wrote:

On November 26, 2016 8:06:26 AM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Nov 25, 2016 at 11:12 PM, Andres Freund <andres@anarazel.de>

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

Andres Freund <andres@anarazel.de> writes:

On 2016-11-26 08:41:28 -0800, Andres Freund wrote:

On November 26, 2016 8:06:26 AM PST, Tom Lane <tgl@sss.pgh.pa.us> 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

On 2017-02-14 17:58:23 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2016-11-26 08:41:28 -0800, Andres Freund wrote:

On November 26, 2016 8:06:26 AM PST, Tom Lane <tgl@sss.pgh.pa.us> 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