On 11/1/2024 18:30, Alexander Korotkov wrote:
Hi!
On Tue, Jan 9, 2024 at 1:14 PM Pavel Borisov <pashkin.e...@gmail.com> wrote:
Hmm, I don't see this old code in these patches. Resend 0002-* because
of trailing spaces.
AFAIK, cfbot does not seek old versions of patchset parts in previous messages.
So for it to run correctly, a new version of the whole patchset should be sent
even if most patches are unchanged.
Please, find the revised patchset with some refactoring and comments
improvement from me. I'll continue looking into this.
The patch looks better, thanks to your refactoring.
I propose additional comments and tests to make the code more
understandable (see attachment).
I intended to look into this part of the code more, but the tests show a
difference between PostgreSQL v.15 and v.16, which causes changes in the
code of this feature.
--
regards,
Andrei Lepikhov
Postgres Professional
diff --git a/src/backend/optimizer/path/pathkeys.c
b/src/backend/optimizer/path/pathkeys.c
index f4b7dcac21..5aac6d6677 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -397,6 +397,11 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List
**group_pathkeys,
!list_member_ptr(*group_pathkeys, pathkey))
break;
+ /*
+ * Since 1349d27 pathkey coming from underlying node can be in
the
+ * root->group_pathkeys but not in the processed_groupClause.
So, we
+ * should be careful here.
+ */
sgc =
get_sortgroupref_clause_noerr(pathkey->pk_eclass->ec_sortref,
*group_clauses);
if (!sgc)
diff --git a/src/test/regress/expected/aggregates.out
b/src/test/regress/expected/aggregates.out
index 423c8ec3b6..0d46e096e5 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -2857,6 +2857,28 @@ explain (COSTS OFF) SELECT x,w FROM btg GROUP BY w,x,y,z
ORDER BY x*x,z;
(8 rows)
DROP TABLE btg;
+-- The case, when scanning sort order correspond to aggregate sort order but
+-- can not be found in the group-by list
+CREATE TABLE t1 (c1 int PRIMARY KEY, c2 int);
+CREATE UNIQUE INDEX ON t1(c2);
+explain (costs off)
+SELECT array_agg(c1 ORDER BY c2),c2
+FROM t1 WHERE c2 < 100 GROUP BY c1 ORDER BY 2;
+ QUERY PLAN
+--------------------------------------------------------
+ Sort
+ Sort Key: c2
+ -> GroupAggregate
+ Group Key: c1
+ -> Sort
+ Sort Key: c1, c2
+ -> Bitmap Heap Scan on t1
+ Recheck Cond: (c2 < 100)
+ -> Bitmap Index Scan on t1_c2_idx
+ Index Cond: (c2 < 100)
+(10 rows)
+
+DROP TABLE t1 CASCADE;
RESET enable_hashagg;
RESET max_parallel_workers;
RESET max_parallel_workers_per_gather;
diff --git a/src/test/regress/sql/aggregates.sql
b/src/test/regress/sql/aggregates.sql
index b9fcceedd7..f99167ac9e 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -1224,6 +1224,15 @@ explain (COSTS OFF) SELECT x,w FROM btg GROUP BY w,x,y,z
ORDER BY x*x,z;
DROP TABLE btg;
+-- The case, when scanning sort order correspond to aggregate sort order but
+-- can not be found in the group-by list
+CREATE TABLE t1 (c1 int PRIMARY KEY, c2 int);
+CREATE UNIQUE INDEX ON t1(c2);
+explain (costs off)
+SELECT array_agg(c1 ORDER BY c2),c2
+FROM t1 WHERE c2 < 100 GROUP BY c1 ORDER BY 2;
+DROP TABLE t1 CASCADE;
+
RESET enable_hashagg;
RESET max_parallel_workers;
RESET max_parallel_workers_per_gather;
