Horton

Thread

Commits

  1. Make some subquery-using test cases a bit more robust.

  2. Prevent generation of bogus subquery scan paths.

  1. Soon-to-be-broken regression test case

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-10-11T16:34:28Z

    The last test case in select_parallel.sql, added in commit dc1057fc,
currently generates a plan like this:

CREATE VIEW tenk1_vw_sec WITH (security_barrier) AS SELECT * FROM tenk1;
EXPLAIN (COSTS OFF)
SELECT 1 FROM tenk1_vw_sec WHERE EXISTS (SELECT 1 WHERE unique1 = 0);
                            QUERY PLAN                             
-------------------------------------------------------------------
 Subquery Scan on tenk1_vw_sec
   Filter: (alternatives: SubPlan 1 or hashed SubPlan 2)
   ->  Gather
         Workers Planned: 4
         ->  Parallel Index Only Scan using tenk1_unique1 on tenk1
   SubPlan 1
     ->  Result
           One-Time Filter: (tenk1_vw_sec.unique1 = 0)
   SubPlan 2
     ->  Result
(10 rows)

I have been fooling around with a patch to allow pull-up of sub-selects
that lack any FROM, along the lines discussed in
https://www.postgresql.org/message-id/15944.1521127664@sss.pgh.pa.us
I find that it is smart enough to reduce that EXISTS to a plain
expression, yielding

                     QUERY PLAN                     
----------------------------------------------------
 Subquery Scan on tenk1_vw_sec
   ->  Index Only Scan using tenk1_unique1 on tenk1
         Index Cond: (unique1 = 0)
(3 rows)

While that's obviously a far better plan, it does not meet this test
case's stated goal of testing the interaction of subqueries with
parallel query.  Could you suggest a less trivial subquery that will
still do what you intended?

			regards, tom lane

  2. Re: Soon-to-be-broken regression test case

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-10-11T16:39:52Z

    On 2018-Oct-11, Tom Lane wrote:

> I have been fooling around with a patch to allow pull-up of sub-selects
> that lack any FROM, along the lines discussed in
> https://www.postgresql.org/message-id/15944.1521127664@sss.pgh.pa.us
> I find that it is smart enough to reduce that EXISTS to a plain
> expression, yielding
> 
>                      QUERY PLAN                     
> ----------------------------------------------------
>  Subquery Scan on tenk1_vw_sec
>    ->  Index Only Scan using tenk1_unique1 on tenk1
>          Index Cond: (unique1 = 0)
> (3 rows)

Hmm, I have the feeling that this would nullify some tests in
partition_prune also, which IIRC pretend to invoke runtime pruning with
things like "WHERE partcol = (select 1)".

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  3. Re: Soon-to-be-broken regression test case

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-10-11T16:45:24Z

    Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2018-Oct-11, Tom Lane wrote:
>> I have been fooling around with a patch to allow pull-up of sub-selects
>> that lack any FROM, along the lines discussed in
>> https://www.postgresql.org/message-id/15944.1521127664@sss.pgh.pa.us
>> I find that it is smart enough to reduce that EXISTS to a plain
>> expression, yielding
>> 
>>                     QUERY PLAN                     
>> ----------------------------------------------------
>> Subquery Scan on tenk1_vw_sec
>>   ->  Index Only Scan using tenk1_unique1 on tenk1
>>         Index Cond: (unique1 = 0)
>> (3 rows)

> Hmm, I have the feeling that this would nullify some tests in
> partition_prune also, which IIRC pretend to invoke runtime pruning with
> things like "WHERE partcol = (select 1)".

Hm, I'm not seeing any regression test result changes there.  However,
if you're just executing queries and not EXPLAIN'ing them, it's possible
something unwanted is happening under the hood.

			regards, tom lane

  4. Re: Soon-to-be-broken regression test case

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-10-11T16:52:51Z

    On 2018-Oct-11, Tom Lane wrote:

> Hm, I'm not seeing any regression test result changes there.  However,
> if you're just executing queries and not EXPLAIN'ing them, it's possible
> something unwanted is happening under the hood.

Hmm, no, the explains are there.  Here's one example -- maybe your new
planner smarts do not change these plans for some reason (I note that
you mentioned EXISTS in your OP, which this one does not use; I further
note that we don't use EXISTS anywhere in partition_prune.sql, which
probably amounts to uncovered cases):

prepare ab_q2 (int, int) as
select a from ab where a between $1 and $2 and b < (select 3);

explain (analyze, costs off, summary off, timing off) execute ab_q2 (2, 2);
                       QUERY PLAN                       
--------------------------------------------------------
 Append (actual rows=0 loops=1)
   InitPlan 1 (returns $0)
     ->  Result (actual rows=1 loops=1)
   Subplans Removed: 6
   ->  Seq Scan on ab_a2_b1 (actual rows=0 loops=1)
         Filter: ((a >= $1) AND (a <= $2) AND (b < $0))
   ->  Seq Scan on ab_a2_b2 (actual rows=0 loops=1)
         Filter: ((a >= $1) AND (a <= $2) AND (b < $0))
   ->  Seq Scan on ab_a2_b3 (never executed)
         Filter: ((a >= $1) AND (a <= $2) AND (b < $0))
(10 rows)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  5. Re: Soon-to-be-broken regression test case

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-10-11T17:19:11Z

    Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2018-Oct-11, Tom Lane wrote:
>> Hm, I'm not seeing any regression test result changes there.  However,
>> if you're just executing queries and not EXPLAIN'ing them, it's possible
>> something unwanted is happening under the hood.

> Hmm, no, the explains are there.  Here's one example -- maybe your new
> planner smarts do not change these plans for some reason

Oh, I see --- these are just "scalar-result sub-SELECTs", not
sub-select-in-FROM, so they never get into the join tree to begin with.
WHERE EXISTS is an exception because we attempt to translate it to a
JOIN_SEMI join, exposing an opportunity for subquery pullup.

			regards, tom lane

  6. Re: Soon-to-be-broken regression test case

    David Rowley <david.rowley@2ndquadrant.com> — 2018-10-11T19:45:37Z

    On 12 October 2018 at 05:52, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2018-Oct-11, Tom Lane wrote:
>
>> Hm, I'm not seeing any regression test result changes there.  However,
>> if you're just executing queries and not EXPLAIN'ing them, it's possible
>> something unwanted is happening under the hood.
>
> Hmm, no, the explains are there.  Here's one example -- maybe your new
> planner smarts do not change these plans for some reason (I note that
> you mentioned EXISTS in your OP, which this one does not use; I further
> note that we don't use EXISTS anywhere in partition_prune.sql, which
> probably amounts to uncovered cases):
>
> prepare ab_q2 (int, int) as
> select a from ab where a between $1 and $2 and b < (select 3);

I guess if we ever did something to break that then we'd need to not
do anything when there are volatile functions present.  If people are
writing that then probably they're doing so to trick the planner,
perhaps to hide some stats that get outdated easily. I'd imagine we'd
upset more people than we'd please.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  7. Re: Soon-to-be-broken regression test case

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-10-11T20:45:44Z

    David Rowley <david.rowley@2ndquadrant.com> writes:
> I guess if we ever did something to break that then we'd need to not
> do anything when there are volatile functions present.

Yeah, nothing I'm doing here changes the rule that we don't flatten
sub-selects containing volatiles in their tlist.

> If people are
> writing that then probably they're doing so to trick the planner,
> perhaps to hide some stats that get outdated easily. I'd imagine we'd
> upset more people than we'd please.

The specific case I'm aware of is that people sometimes write
"(SELECT x)" rather than just "x" so as to make the calculation
be a done-only-once InitPlan.  That code path isn't affected by
this, either (and that's why the partition_prune tests didn't
change behavior).

It's fair to wonder whether partition_prune needs to be testing
other subplan cases besides InitPlans, but that seems like a
distinct issue.

			regards, tom lane