diff --git a/src/backend/columnar/columnar_reader.c b/src/backend/columnar/columnar_reader.c index cfd052ac2..96efdbffc 100644 --- a/src/backend/columnar/columnar_reader.c +++ b/src/backend/columnar/columnar_reader.c @@ -81,6 +81,13 @@ struct ColumnarReadState MemoryContext stripeReadContext; int64 chunkGroupsFiltered; + + /* + * Memory context guaranteed to be not freed during scan so we can + * safely use for any memory allocations regarding ColumnarReadState + * itself. + */ + MemoryContext scanContext; }; /* static function declarations */ @@ -159,7 +166,8 @@ static Datum ColumnDefaultValue(TupleConstr *tupleConstraints, */ ColumnarReadState * ColumnarBeginRead(Relation relation, TupleDesc tupleDescriptor, - List *projectedColumnList, List *whereClauseList) + List *projectedColumnList, List *whereClauseList, + MemoryContext scanContext) { /* * We allocate all stripe specific data in the stripeReadContext, and reset @@ -180,6 +188,7 @@ ColumnarBeginRead(Relation relation, TupleDesc tupleDescriptor, readState->currentStripeMetadata = FindNextStripeByRowNumber(relation, COLUMNAR_INVALID_ROW_NUMBER, GetTransactionSnapshot()); + readState->scanContext = scanContext; return readState; } @@ -432,11 +441,15 @@ HasUnreadStripe(ColumnarReadState *readState) void ColumnarRescan(ColumnarReadState *readState) { + MemoryContext oldContext = MemoryContextSwitchTo(readState->scanContext); + ColumnarResetRead(readState); readState->currentStripeMetadata = FindNextStripeByRowNumber(readState->relation, COLUMNAR_INVALID_ROW_NUMBER, GetTransactionSnapshot()); readState->chunkGroupsFiltered = 0; + + MemoryContextSwitchTo(oldContext); } @@ -518,6 +531,8 @@ BeginStripeRead(StripeMetadata *stripeMetadata, Relation rel, TupleDesc tupleDes static void AdvanceStripeRead(ColumnarReadState *readState) { + MemoryContext oldContext = MemoryContextSwitchTo(readState->scanContext); + readState->chunkGroupsFiltered += readState->stripeReadState->chunkGroupsFiltered; @@ -528,6 +543,8 @@ AdvanceStripeRead(ColumnarReadState *readState) GetTransactionSnapshot()); readState->stripeReadState = NULL; MemoryContextReset(readState->stripeReadContext); + + MemoryContextSwitchTo(oldContext); } diff --git a/src/backend/columnar/columnar_tableam.c b/src/backend/columnar/columnar_tableam.c index 532613dd4..6630d0430 100644 --- a/src/backend/columnar/columnar_tableam.c +++ b/src/backend/columnar/columnar_tableam.c @@ -253,11 +253,15 @@ CreateColumnarScanMemoryContext(void) */ static ColumnarReadState * init_columnar_read_state(Relation relation, TupleDesc tupdesc, Bitmapset *attr_needed, - List *scanQual) + List *scanQual, MemoryContext scanContext) { + MemoryContext oldContext = MemoryContextSwitchTo(scanContext); + List *neededColumnList = NeededColumnsList(tupdesc, attr_needed); ColumnarReadState *readState = ColumnarBeginRead(relation, tupdesc, neededColumnList, - scanQual); + scanQual, scanContext); + + MemoryContextSwitchTo(oldContext); return readState; } @@ -302,11 +306,10 @@ columnar_getnextslot(TableScanDesc sscan, ScanDirection direction, TupleTableSlo */ if (scan->cs_readState == NULL) { - MemoryContext oldContext = MemoryContextSwitchTo(scan->scanContext); scan->cs_readState = init_columnar_read_state(scan->cs_base.rs_rd, slot->tts_tupleDescriptor, - scan->attr_needed, scan->scanQual); - MemoryContextSwitchTo(oldContext); + scan->attr_needed, scan->scanQual, + scan->scanContext); } ExecClearTuple(slot); @@ -490,8 +493,6 @@ columnar_index_fetch_tuple(struct IndexFetchTableData *sscan, /* initialize read state for the first row */ if (scan->cs_readState == NULL) { - MemoryContext oldContext = MemoryContextSwitchTo(scan->scanContext); - /* we need all columns */ int natts = columnarRelation->rd_att->natts; Bitmapset *attr_needed = bms_add_range(NULL, 0, natts - 1); @@ -501,8 +502,8 @@ columnar_index_fetch_tuple(struct IndexFetchTableData *sscan, scan->cs_readState = init_columnar_read_state(columnarRelation, slot->tts_tupleDescriptor, - attr_needed, scanQual); - MemoryContextSwitchTo(oldContext); + attr_needed, scanQual, + scan->scanContext); } uint64 rowNumber = tid_to_row_number(*tid); @@ -795,10 +796,14 @@ columnar_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, /* we need all columns */ int natts = OldHeap->rd_att->natts; Bitmapset *attr_needed = bms_add_range(NULL, 0, natts - 1); - List *projectedColumnList = NeededColumnsList(sourceDesc, attr_needed); - ColumnarReadState *readState = ColumnarBeginRead(OldHeap, sourceDesc, - projectedColumnList, - NULL); + + /* no quals for table rewrite */ + List *scanQual = NIL; + + MemoryContext scanContext = CreateColumnarScanMemoryContext(); + ColumnarReadState *readState = init_columnar_read_state(OldHeap, sourceDesc, + attr_needed, scanQual, + scanContext); Datum *values = palloc0(sourceDesc->natts * sizeof(Datum)); bool *nulls = palloc0(sourceDesc->natts * sizeof(bool)); diff --git a/src/include/columnar/columnar.h b/src/include/columnar/columnar.h index eaeb9fa21..a44072ffe 100644 --- a/src/include/columnar/columnar.h +++ b/src/include/columnar/columnar.h @@ -212,7 +212,8 @@ extern MemoryContext ColumnarWritePerTupleContext(ColumnarWriteState *state); extern ColumnarReadState * ColumnarBeginRead(Relation relation, TupleDesc tupleDescriptor, List *projectedColumnList, - List *qualConditions); + List *qualConditions, + MemoryContext scanContext); extern bool ColumnarReadNextRow(ColumnarReadState *state, Datum *columnValues, bool *columnNulls, uint64 *rowNumber); extern void ColumnarRescan(ColumnarReadState *readState); diff --git a/src/test/regress/expected/columnar_alter.out b/src/test/regress/expected/columnar_alter.out index 707107568..2cf31a86b 100644 --- a/src/test/regress/expected/columnar_alter.out +++ b/src/test/regress/expected/columnar_alter.out @@ -218,6 +218,21 @@ SELECT * FROM domain_test; 1 | 2 | x (2 rows) +-- similar to "add column c str_domain DEFAULT 'x'", both were getting +-- stucked before fixing https://github.com/citusdata/citus/issues/5164 +BEGIN; + ALTER TABLE domain_test ADD COLUMN d INT DEFAULT random(); +ROLLBACK; +BEGIN; + ALTER TABLE domain_test ADD COLUMN d SERIAL; + SELECT * FROM domain_test ORDER BY 1,2,3,4; + a | b | c | d +--------------------------------------------------------------------- + 1 | 2 | x | 1 + 1 | 2 | x | 2 +(2 rows) + +ROLLBACK; set default_table_access_method TO 'columnar'; CREATE TABLE has_volatile AS SELECT * FROM generate_series(1,10) id; diff --git a/src/test/regress/sql/columnar_alter.sql b/src/test/regress/sql/columnar_alter.sql index ecd647fe1..7872773d7 100644 --- a/src/test/regress/sql/columnar_alter.sql +++ b/src/test/regress/sql/columnar_alter.sql @@ -116,6 +116,16 @@ alter table domain_test add column c str_domain; alter table domain_test add column c str_domain DEFAULT 'x'; SELECT * FROM domain_test; +-- similar to "add column c str_domain DEFAULT 'x'", both were getting +-- stucked before fixing https://github.com/citusdata/citus/issues/5164 +BEGIN; + ALTER TABLE domain_test ADD COLUMN d INT DEFAULT random(); +ROLLBACK; +BEGIN; + ALTER TABLE domain_test ADD COLUMN d SERIAL; + SELECT * FROM domain_test ORDER BY 1,2,3,4; +ROLLBACK; + set default_table_access_method TO 'columnar'; CREATE TABLE has_volatile AS SELECT * FROM generate_series(1,10) id;