Skip to content

GG-399: Enable incremental sort for greengage#2480

Open
dkovalev1 wants to merge 24 commits into
adb-8.xfrom
GG-399
Open

GG-399: Enable incremental sort for greengage#2480
dkovalev1 wants to merge 24 commits into
adb-8.xfrom
GG-399

Conversation

@dkovalev1

@dkovalev1 dkovalev1 commented May 7, 2026

Copy link
Copy Markdown

Enable Incremental Sort for Greengage

PostgreSQL implemented the incremental sort algorithm in commit d2d8a22.
Greengage included this code in commit 2feb2cc but disabled it, as incremental
sort required adaptation to Greengage-specific code.

This commit enables incremental sort for Greengage:

  • Enable the enable_incremental_sort planner parameter by default for
    Greengage deployments
  • Add the incremental sort node to the Greengage-specific parts of the Postgres
    planner
  • Add incremental sort support to the plan serializer
  • Add support for the squelch mode
  • Adapt test output for distributed plans
  • Adapt tests to exclude non-deterministic row order expectations

GG-399

@dkovalev1 dkovalev1 marked this pull request as draft May 7, 2026 15:04
@RekGRpth

This comment was marked as resolved.

@dkovalev1 dkovalev1 changed the title [DRAFT] GG-399: Enable incremental sort for greengage GG-399: Enable incremental sort for greengage May 12, 2026
@dkovalev1 dkovalev1 marked this pull request as ready for review May 12, 2026 06:55
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c Outdated

@KnightMurloc KnightMurloc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about switches in motion_sanity_walker, DirectDispatchUpdateContentIdsFromPlan, IsBlockingOperator and IsMemoryIntensiveOperator? Also not ggdb specific, but strange that we don't have a case for Incremental sort here: ExecMaterializesOutput, plannode_type and is_projection_capable_plan.

Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/test/regress/expected/incremental_sort.out
Comment thread src/test/regress/expected/incremental_sort.out Outdated
Comment thread src/test/regress/expected/incremental_sort.out Outdated
Comment thread src/test/regress/expected/incremental_sort.out Outdated
@KnightMurloc

Copy link
Copy Markdown

We get wrong result if table is not distributed by group key. The Redistribute Motion is not maintain the order.

-- t2 is randomly distributed
explain select a, b, count(*) as res from t2 group by a, b;
                                                QUERY PLAN                                                 
-----------------------------------------------------------------------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)  (cost=0.40..3451.65 rows=10009 width=16)
   ->  GroupAggregate  (cost=0.40..3318.19 rows=3336 width=16)
         Group Key: a, b
         ->  Incremental Sort  (cost=0.40..3034.83 rows=33333 width=8)
               Sort Key: a, b
               Presorted Key: a
               ->  Redistribute Motion 3:3  (slice2; segments: 3)  (cost=0.17..1796.16 rows=33333 width=8)
                     Hash Key: a, b
                     ->  Index Scan using t2_a_idx on t2  (cost=0.17..1129.49 rows=33333 width=8)
 Optimizer: Postgres query optimizer
(10 rows)

Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/test/regress/expected/incremental_sort.out Outdated
@dkovalev1

Copy link
Copy Markdown
Author

We get wrong result if table is not distributed by group key. The Redistribute Motion is not maintain the order.

-- t2 is randomly distributed
explain select a, b, count(*) as res from t2 group by a, b;
                                                QUERY PLAN                                                 
-----------------------------------------------------------------------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)  (cost=0.40..3451.65 rows=10009 width=16)
   ->  GroupAggregate  (cost=0.40..3318.19 rows=3336 width=16)
         Group Key: a, b
         ->  Incremental Sort  (cost=0.40..3034.83 rows=33333 width=8)
               Sort Key: a, b
               Presorted Key: a
               ->  Redistribute Motion 3:3  (slice2; segments: 3)  (cost=0.17..1796.16 rows=33333 width=8)
                     Hash Key: a, b
                     ->  Index Scan using t2_a_idx on t2  (cost=0.17..1129.49 rows=33333 width=8)
 Optimizer: Postgres query optimizer
(10 rows)

could you share complete steps to reproduce? I am getting different plan.

@KnightMurloc

Copy link
Copy Markdown

could you share complete steps to reproduce? I am getting different plan.

create table t1 (a int, b int) distributed randomly;
create index ON t1 (a);
-- 10 times
insert into t1 select a, a from generate_series(1, 10000);
analyze t1;
set enable_hashagg = off;
set optimizer = off;
explain select a, b, count(*) as res from t1 group by a, b;

@RekGRpth

This comment was marked as resolved.

@dkovalev1

Copy link
Copy Markdown
Author

could you share complete steps to reproduce? I am getting different plan.

create table t1 (a int, b int) distributed randomly;
create index ON t1 (a);
-- 10 times
insert into t1 select a, a from generate_series(1, 10000);
analyze t1;
set enable_hashagg = off;
set optimizer = off;
explain select a, b, count(*) as res from t1 group by a, b;

Thanks, test added, fix applied

Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/test/regress/sql/incremental_sort.sql Outdated
Comment thread src/test/regress/expected/incremental_sort.out
Comment thread src/backend/cdb/cdbgroupingpaths.c
Comment thread src/backend/cdb/cdbgroupingpaths.c Outdated
Comment thread src/backend/executor/nodeIncrementalSort.c
Comment thread src/backend/executor/nodeIncrementalSort.c
Comment thread src/backend/executor/nodeIncrementalSort.c
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c Outdated
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/backend/optimizer/plan/planner.c
Comment thread src/include/nodes/execnodes.h
@RekGRpth

RekGRpth commented Jun 2, 2026

Copy link
Copy Markdown
Member

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants