On Feb 18, 2025 at 16:17 +0800, Japin Li <japi...@hotmail.com>, wrote: > On Tue, 18 Feb 2025 at 15:40, "songjinzhou" <tsinghualucky...@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! > > Hi,
Good catch. > > 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? While there isn't an unique index, an unique constraint with a NOT NULL on that column should serve the same purpose. +alter table t2 alter column z set not null, add constraint t2_z_uidx unique (z); In patch v2, I made a slight adjustment: creating a unique index on t3 will handle multiple choice scenarios. Thanks for the patch. -- Zhang Mingli HashData
v2-0001-Modify-an-incorrect-regression-test-case-in-the-grou.patch
Description: Binary data
