public inbox for [email protected]
help / color / mirror / Atom feedFrom: Japin Li <[email protected]>
To: songjinzhou <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: David Rowley <[email protected]>
Subject: Re: Modify an incorrect regression test case in the group by key value elimination function
Date: Tue, 18 Feb 2025 16:17:25 +0800
Message-ID: <ME0P300MB04455DD194CE6F5A8CAFAD85B6FA2@ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
On Tue, 18 Feb 2025 at 15:40, "songjinzhou" <[email protected]> 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
view thread (5+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Modify an incorrect regression test case in the group by key value elimination function
In-Reply-To: <ME0P300MB04455DD194CE6F5A8CAFAD85B6FA2@ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox