Improving planner's checks for parallel-unsafety

On Thu, Aug 18, 2016 at 12:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Attached is a patch I'd fooled around with back in July but not submitted.
The idea is that, if our initial scan of the query tree found only
parallel-safe functions, there is no need to rescan subsets of the tree
looking for parallel-restricted functions. We can mechanize that by
saving the "maximum unsafety" level in PlannerGlobal and looking aside
at that value before conducting a check of a subset of the tree.

This is not a huge win, but it's measurable. I see about 3% overall TPS
improvement in pgbench on repeated execution of this test query:

select
abs(unique1) + abs(unique1),
abs(unique2) + abs(unique2),
abs(two) + abs(two),
abs(four) + abs(four),
abs(ten) + abs(ten),
abs(twenty) + abs(twenty),
abs(hundred) + abs(hundred),
abs(thousand) + abs(thousand),
abs(twothousand) + abs(twothousand),
abs(fivethous) + abs(fivethous),
abs(tenthous) + abs(tenthous),
abs(odd) + abs(odd),
abs(even) + abs(even)
from tenk1 limit 1;

This test case is admittedly a bit contrived, in that the number of
function calls that have to be checked is high relative to both the
planning cost and execution cost of the query. Still, the fact that
the difference is above the noise floor even in an end-to-end test
says that the current method of checking functions twice is pretty
inefficient.

I'll put this in the commitfest queue.

I have reviewed this and it looks good to me. My only comment is that
this comment is slightly confusing:

! * Returns the first of PROPARALLEL_UNSAFE, PROPARALLEL_RESTRICTED, or
! * PROPARALLEL_SAFE that can be found in the given parsetree. We use this

"First" might be read to mean "the first one we happen to run across"
rather than "the earliest in list ordering".

--
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:

I have reviewed this and it looks good to me. My only comment is that
this comment is slightly confusing:

! * Returns the first of PROPARALLEL_UNSAFE, PROPARALLEL_RESTRICTED, or
! * PROPARALLEL_SAFE that can be found in the given parsetree. We use this

"First" might be read to mean "the first one we happen to run across"
rather than "the earliest in list ordering".

Thanks for the review. I'll reconsider how to phrase that --- have you
any suggestions?

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 Thu, Aug 18, 2016 at 5:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

I have reviewed this and it looks good to me. My only comment is that
this comment is slightly confusing:

! * Returns the first of PROPARALLEL_UNSAFE, PROPARALLEL_RESTRICTED, or
! * PROPARALLEL_SAFE that can be found in the given parsetree. We use this

"First" might be read to mean "the first one we happen to run across"
rather than "the earliest in list ordering".

Thanks for the review. I'll reconsider how to phrase that --- have you
any suggestions?

I think what you committed is fine.

--
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