Skip to content

columnar: serialize CREATE INDEX on columnar tables for PG19 #8618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ihalatci wants to merge 1 commit into
base: pg19-support
Choose a base branch
from
+50 −8
Open

columnar: serialize CREATE INDEX on columnar tables for PG19 #8618

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 50 additions & 8 deletions src/backend/columnar/columnar_tableam.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,24 +412,33 @@ ErrorIfInvalidRowNumber(uint64 rowNumber)
}


/*
* Columnar scans are always serial regardless of the parallel scan descriptor
* (rs_parallel is stored but never read by the columnar scan path). We still
* provide working parallelscan callbacks so that callers that unconditionally
* size and initialize a parallel scan descriptor (for example, PG19's parallel
* btree index build path) do not abort. Delegating to the heap block helpers
* is safe: columnar never partitions work using the descriptor, so any state
* written there is simply unused.
*/
static Size
columnar_parallelscan_estimate(Relation rel)
{
elog(ERROR, "columnar_parallelscan_estimate not implemented");
return sizeof(ParallelBlockTableScanDescData);
}
Comment on lines 424 to 428


static Size
columnar_parallelscan_initialize(Relation rel, ParallelTableScanDesc pscan)
{
elog(ERROR, "columnar_parallelscan_initialize not implemented");
return table_block_parallelscan_initialize(rel, pscan);
}


static void
columnar_parallelscan_reinitialize(Relation rel, ParallelTableScanDesc pscan)
{
elog(ERROR, "columnar_parallelscan_reinitialize not implemented");
table_block_parallelscan_reinitialize(rel, pscan);
}


Expand Down Expand Up @@ -1528,10 +1537,13 @@ columnar_index_build_range_scan(Relation columnarRelation,
if (scan)
{
/*
* Parallel scans on columnar tables are already discardad by
* ColumnarGetRelationInfoHook but be on the safe side.
* Starting with PG19 the planner may hand us a parallel scan descriptor
* even for tables (like columnar) whose AM only supports serial scans.
* Discard it: columnar never partitions index-build work across workers
* and always starts its own serial scan below. This mirrors how the
* existing serial path ignores any externally supplied scan.
*/
elog(ERROR, "parallel scans on columnar are not supported");
scan = NULL;
}

/*
Expand Down Expand Up @@ -2315,6 +2327,7 @@ ColumnarProcessUtility(PlannedStmt *pstmt,

RangeVar *columnarRangeVar = NULL;
List *columnarOptions = NIL;
bool indexBuildOnColumnar = false;

switch (nodeTag(parsetree))
{
Expand All @@ -2337,6 +2350,15 @@ ColumnarProcessUtility(PlannedStmt *pstmt,
"index on columnar table %s",
RelationGetRelationName(rel))));
}

/*
* Columnar does not support parallel index build (its scan path is
* always serial and writes during the build would violate parallel
* mode). Force max_parallel_maintenance_workers to 0 for this
* statement so PG does not spawn workers. Starting with PG19 the
* default of 2 caused CREATE INDEX on a columnar table to fail.
*/
indexBuildOnColumnar = true;
}

RelationClose(rel);
Expand Down Expand Up @@ -2427,8 +2449,28 @@ ColumnarProcessUtility(PlannedStmt *pstmt,
CheckCitusColumnarAlterExtensionStmt(parsetree);
}

PrevProcessUtilityHook(pstmt, queryString, false, context,
params, queryEnv, dest, completionTag);
int saveNestLevel = -1;
if (indexBuildOnColumnar)
{
saveNestLevel = NewGUCNestLevel();
(void) set_config_option("max_parallel_maintenance_workers", "0",

@colm-mchugh colm-mchugh Jun 18, 2026

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.

nit: (void) goes against existing calls of set_config_option() (e.g. in PostprocessReassignOwnedStmt(), citus_internal_database_command())

PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false);

@colm-mchugh colm-mchugh Jun 18, 2026 •
edited
Loading

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.

Should the removal of parallelism also be applied for REINDEX statements ? They have a separate node tag, T_ReindexStmt, which is not directly handled by this switch statement afaict, so falls to the default branch. As is, would a REINDEX of a columnar table with pending writes get parallel mode and hit the failure ?

}

PG_TRY();
{
PrevProcessUtilityHook(pstmt, queryString, false, context,
params, queryEnv, dest, completionTag);
}
PG_FINALLY();
{
if (saveNestLevel >= 0)
{
AtEOXact_GUC(true, saveNestLevel);
}
}
PG_END_TRY();

@colm-mchugh colm-mchugh Jun 18, 2026

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.

Wondering if the current tests in columnar_index.sql are enough to validate the fixes ?
A couple of test suggestions:

  1. set max_parallel_maintenance_workers to e..g 4 before a CREATE INDEX on a columnar table, and then show that it is still 4 after the CREATE INDEX.
  2. Similar GUC setting for a REINDEX operation;
SET LOCAL debug_parallel_query = regress;
SET LOCAL max_parallel_workers = 4;
REINDEX TABLE parallel_scan_test;
REINDEX TABLE CONCURRENTLY parallel_scan_test;
SHOW LOCAL max_parallel_workers;


if (columnarOptions != NIL)
{
Expand Down
Loading