Thread: Re: Modify an incorrect regression test case in the group by key value elimination function

On Tue, 18 Feb 2025 at 15:40, "songjinzhou" <tsinghualucky912@foxmail.com> wrote:
> Dear hackers: 
>
> Two months ago, we enhanced the group by key value elimination function. This is a very useful function. When I was
> learning, I found a regression test case that was not suitable, as follows:
>
> -- When there are multiple supporting unique indexes and the GROUP BY contains
> -- columns to cover all of those, ensure we pick the index with the least
> -- number of columns so that we can remove more columns from the GROUP BY.
> explain (costs off) select a,b,c from t3 group by a,b,c;
>       QUERY PLAN      
> ----------------------
>  HashAggregate
>    Group Key: c
>    ->  Seq Scan on t3
> (3 rows)
>
> -- As above but try ordering the columns differently to ensure we get the
> -- same result.
> explain (costs off) select a,b,c from t3 group by c,a,b;
>       QUERY PLAN      
> ----------------------
>  HashAggregate
>    Group Key: c
>    ->  Seq Scan on t3
> (3 rows)
>
> Because the table structure of t3 is defined as follows(Its PK is deferrable):
>
> postgres=# \d+ t3
>                                           Table "pg_temp_1.t3"
>  Column |  Type   | Collation | Nullable | Default | Storage | Compression | Stats target | Description
> --------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
>  a      | integer |           | not null |         | plain   |             |              |
>  b      | integer |           | not null |         | plain   |             |              |
>  c      | integer |           | not null |         | plain   |             |              |
> Indexes:
>     "t3_pkey" PRIMARY KEY, btree (a, b) DEFERRABLE
>     "t3_c_uidx" UNIQUE, btree (c)
> Not-null constraints:
>     "t3_a_not_null" NOT NULL "a"
>     "t3_b_not_null" NOT NULL "b"
>     "t3_c_not_null" NOT NULL "c"
> Access method: heap
>
> postgres=#
>
> I think this test case does not fully reflect the original meaning. So I made a small change to this and look forward
to
> your feedback. Thanks!
>

Yeah, the primary key of t3 is deferred, so only the t3_c_uidx can be used.
The test case is inconsistent with comments.

It seems that the t2 does not have a unique index on z, do you forget to
create it in the patch?

-- 
Regrads,
Japin Li