Re: [HACKERS] Improving planner's checks for parallel-unsafety

[Robert Haas]

On Thu, Aug 18, 2016 at 5:07 PM, Tom Lane  wrote:
> Robert Haas  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

Re: [HACKERS] Improving planner's checks for parallel-unsafety

[Tom Lane]

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

Re: [HACKERS] Improving planner's checks for parallel-unsafety

[Robert Haas]

On Thu, Aug 18, 2016 at 12:39 PM, Tom Lane  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