Skip to content

Feature: dispatch T_CustomScanState in CBDB parallel walkers - #1855

Open
roseduan wants to merge 3 commits into
apache:mainfrom
roseduan:feature/dispatch-customscanstate-parallel-walkers
Open

Feature: dispatch T_CustomScanState in CBDB parallel walkers#1855
roseduan wants to merge 3 commits into
apache:mainfrom
roseduan:feature/dispatch-customscanstate-parallel-walkers

Conversation

@roseduan

Copy link
Copy Markdown
Contributor

Three GP-side walkers used during parallel setup — EstimateGpParallelDSMEntrySize, InitializeGpParallelWorkers, InitializeGpParallelDSMEntry — cased on other scan/join states but skipped T_CustomScanState. A parallel_aware CustomScan therefore silently under-sized its DSM and failed to attach in workers, with no diagnostic.

Add T_CustomScanState arms that dispatch to
ExecCustomScan{Estimate,InitializeDSM,InitializeWorker}, gated on parallel_aware to match the other node types in these switches.

Also handle T_CustomScanState in planstate_walk_kids. Upstream planstate_tree_walker walks only css->custom_ps for a CustomScanState; the CBDB walker previously fell through to default (lefttree/righttree), skipping custom_ps children under stateful walkers (cdbexplain_*, getMotionState, ...). The new case walks custom_ps first, then lefttree/righttree if set, with an Assert that a CustomScanState never populates both — otherwise the child would be walked twice.

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


Comment thread src/backend/executor/execParallel.c
@roseduan
roseduan force-pushed the feature/dispatch-customscanstate-parallel-walkers branch 3 times, most recently from 920933d to cae8b66 Compare August 3, 2026 03:45
Three GP-side walkers used during parallel setup —
EstimateGpParallelDSMEntrySize, InitializeGpParallelWorkers,
InitializeGpParallelDSMEntry — cased on other scan/join states but
skipped T_CustomScanState.  A parallel_aware CustomScan therefore
silently under-sized its DSM and failed to attach in workers, with
no diagnostic.

Add T_CustomScanState arms that dispatch to
ExecCustomScan{Estimate,InitializeDSM,InitializeWorker}, gated on
parallel_aware to match the other node types in these switches.

Also handle T_CustomScanState in planstate_walk_kids. Upstream
planstate_tree_walker walks only css->custom_ps for a
CustomScanState; the CBDB walker previously fell through to
default (lefttree/righttree), skipping custom_ps children under
stateful walkers (cdbexplain_*, getMotionState, ...).  The new
case walks custom_ps first, then lefttree/righttree if set, with
an Assert that a CustomScanState never populates both — otherwise
the child would be walked twice.
@roseduan
roseduan force-pushed the feature/dispatch-customscanstate-parallel-walkers branch from cae8b66 to 4458e32 Compare August 4, 2026 09:55
@roseduan
roseduan requested review from yjhjstz and a lite review from Copilot August 8, 2026 05:46

Copilot AI 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.

Pull request overview

Fixes missing T_CustomScanState handling in Cloudberry’s GP-side parallel setup walkers and CBDB’s planstate child-walking logic, and adds a regression test module to exercise parallel-aware CustomScan behavior.

Changes:

  • Dispatch T_CustomScanState to ExecCustomScan{Estimate,InitializeDSM,InitializeWorker} in CBDB parallel walkers when parallel_aware.
  • Teach planstate_walk_kids() to recurse into CustomScanState->custom_ps (and then left/right if present, with an assert to prevent double-walk).
  • Add parallel_customscan test extension + SQL/expected files and wire it into src/test/modules.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/backend/executor/execParallel.c Adds T_CustomScanState dispatch in GP parallel DSM sizing/init/worker init walkers.
src/backend/executor/execProcnode.c Ensures CBDB planstate walker visits CustomScanState->custom_ps children.
src/test/modules/Makefile Adds parallel_customscan to module subdirs.
src/test/modules/parallel_customscan/Makefile New module build + regression target wiring.
src/test/modules/parallel_customscan/parallel_customscan.c New test extension implementing a parallel-aware CustomScan wrapper.
src/test/modules/parallel_customscan/parallel_customscan.control New extension control metadata.
src/test/modules/parallel_customscan/parallel_customscan--1.0.sql New extension SQL defining pcs_get_hook_calls().
src/test/modules/parallel_customscan/sql/parallel_customscan.sql New regression SQL exercising parallel custom scan + EXPLAIN ANALYZE traversal.
src/test/modules/parallel_customscan/expected/parallel_customscan.out New expected regression output.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@@ -0,0 +1,396 @@
#include "postgres.h"
Comment on lines +14 to +18
# Run against an existing cluster (gpdemo). The cluster must have
# 'parallel_customscan' in shared_preload_libraries so segment backends
# and parallel workers have the CustomScan methods registered:
# gpconfig -c shared_preload_libraries -v "'time_series,parallel_customscan'"
# gpstop -ra
Comment on lines +172 to +173
if (TupIsNull(childslot))
return NULL;

@leborchuk leborchuk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

5 participants