From: Andy Fan <zhihui.fan1213@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrew Gierth <andrew@tao11.riddles.org.uk>
Cc: David Rowley <dgrowleyml@gmail.com>
Cc: pavelsivash@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #16585: Wrong filtering on a COALESCE field after using GROUPING SETS
Date: Fri, 21 Aug 2020 12:15:06 +0800
Message-ID: <CAKU4AWqxP2ePgxf8-RNLXPQVOG7=CbTPbfCbtV1ebiHtMT3L6Q@mail.gmail.com> (raw)
In-Reply-To: <CAKU4AWqbcQcFFTMWbUk73tDWryDuOw4H7m082FcvTfkqgcgn3Q@mail.gmail.com>
References: <16585-9d8c340d23ade8c1@postgresql.org>
<CAApHDvoP02UYOS5H82GfDvF_3X9CvWdbseqq-rjufE3GEYRPPA@mail.gmail.com>
<87mu2qixx5.fsf@news-spur.riddles.org.uk>
<1750212.1597958727@sss.pgh.pa.us>
<87tuwxrnzm.fsf@news-spur.riddles.org.uk>
<CAKU4AWoE+QpHbV_tEyGaz_uxvffJnbHuFRK2fXmfs_bj2fKWww@mail.gmail.com>
<1765056.1597977852@sss.pgh.pa.us>
<CAKU4AWqbcQcFFTMWbUk73tDWryDuOw4H7m082FcvTfkqgcgn3Q@mail.gmail.com>
On Fri, Aug 21, 2020 at 11:50 AM Andy Fan <zhihui.fan1213@gmail.com> wrote:
>
>
> On Fri, Aug 21, 2020 at 10:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Andy Fan <zhihui.fan1213@gmail.com> writes:
>> > On Fri, Aug 21, 2020 at 5:51 AM Andrew Gierth <
>> andrew@tao11.riddles.org.uk>
>> > wrote:
>> >> Unless I'm missing something, it should be safe to reference output
>> >> columns that are not mentioned in any grouping set,
>>
>> > I think such columns usually are aggregation expr, If we want to push
>> down
>> > a qual which reference to an aggregation expr, we have to push down
>> > to having cause, However I am not sure such pushing down really helps.
>>
>> Well, they can either be aggregates, or functions of the grouping
>> columns. You're right that there's not much we can do (today) with
>> restrictions on aggregate outputs, but there can be value in pushing
>> down restrictions on the other sort.
>>
>> As an example, consider the regression database's tenk1 table, and
>> for argument's sake add
>>
>> regression=# create index on tenk1 (abs(hundred));
>> CREATE INDEX
>>
>> Then we can get
>>
>> regression=# explain select * from (select hundred, ten, abs(hundred) a,
>> count(*) c from tenk1 group by 1,2) ss where a = 42;
>> QUERY PLAN
>>
>>
>> ------------------------------------------------------------------------------------
>> HashAggregate (cost=225.98..227.18 rows=96 width=20)
>> Group Key: tenk1.hundred, tenk1.ten
>> -> Bitmap Heap Scan on tenk1 (cost=5.06..225.23 rows=100 width=8)
>> Recheck Cond: (abs(hundred) = 42)
>> -> Bitmap Index Scan on tenk1_abs_idx (cost=0.00..5.04
>> rows=100 width=0)
>> Index Cond: (abs(hundred) = 42)
>> (6 rows)
>>
>> which is a lot cheaper than the pure seqscan you get with no pushed-down
>> condition.
>>
>> One thing that I find curious is that if I alter this example to use
>> grouping sets, say
>>
>> regression=# explain select * from (select hundred, ten, abs(hundred) a,
>> count(*) c from tenk1 group by grouping sets (1,2)) ss where a = 42;
>> QUERY PLAN
>> -----------------------------------------------------------------
>> HashAggregate (cost=495.00..546.65 rows=2 width=20)
>> Hash Key: tenk1.hundred
>> Hash Key: tenk1.ten
>> Filter: (abs(tenk1.hundred) = 42)
>> -> Seq Scan on tenk1 (cost=0.00..445.00 rows=10000 width=8)
>> (5 rows)
>>
>> i.e. it's not seeing the abs() condition as pushable below the
>> aggregation. I'm not quite sure if that's a necessary restriction
>> or a missed optimization.
>>
>> regards, tom lane
>>
>
> Both of the queries can push down the qual "a = 42" to the
> subquery->havingQual
> since we have group-by clause, this method unify the process for
> aggregation call
> and non-aggregation expr. . so it become to
>
> select .. from (select .. from tenk1 group .. having (abs(hundred) = 2);
>
>
> later in the subquery_planner, we will try to pull the having clause to
> where clause.
> then the Q2 failed to do so.
>
> /*
> * In some cases we may want to transfer a HAVING clause into
> WHERE. We
> * cannot do so if the HAVING clause contains aggregates
> (obviously) or
> * volatile functions (since a HAVING clause is supposed to be
> executed
> * only once per group). We also can't do this if there are any
> nonempty
> * grouping sets; moving such a clause into WHERE would
> potentially change
> * the results, if any referenced column isn't present in all the
> grouping
> * sets. (If there are only empty grouping sets, then the HAVING
> clause
> * must be degenerate as discussed below.)
> */
>
> I'm still trying to understand the comment, though.
>
>
>
This should be a correct behavior, we should not push down in the Q2
case. Here is an example:
regression=# create table tgs(a int, b int);
CREATE TABLE
regression=# insert into tgs values(1, 1), (1, 2), (2, 2);
INSERT 0 3
regression=# select * from (select a, b, count(*) from tgs group by
grouping sets((a), (b))) t where b = 1;
a | b | count
---+---+-------
| 1 | 1
(1 row)
regression=# select * from (select a, b, count(*) from tgs group by
grouping sets((a), (b)) having b = 1) t;
a | b | count
---+---+-------
| 1 | 1
(1 row)
regression=# select * from (select a, b, count(*) from tgs where b = 1
group by grouping sets((a), (b)) ) t;
a | b | count
---+---+-------
1 | | 1
| 1 | 1
(2 rows)
At the same time, our optimizer is smart enough to handle the below case
(only 1 set in group sets, which equals
group by).
regression=# explain select * from (select a, b, count(*) from tgs group by
grouping sets((a, b)) ) t where b = 1;
QUERY PLAN
-----------------------------------------------------------------
GroupAggregate (cost=38.44..38.66 rows=11 width=16)
Group Key: tgs.a, tgs.b
-> Sort (cost=38.44..38.47 rows=11 width=8)
Sort Key: tgs.a
-> Seq Scan on tgs (cost=0.00..38.25 rows=11 width=8)
Filter: (b = 1)
(6 rows)
--
Best Regards
Andy Fan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: zhihui.fan1213@gmail.com, tgl@sss.pgh.pa.us, andrew@tao11.riddles.org.uk, dgrowleyml@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16585: Wrong filtering on a COALESCE field after using GROUPING SETS
In-Reply-To: <CAKU4AWqxP2ePgxf8-RNLXPQVOG7=CbTPbfCbtV1ebiHtMT3L6Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox