From 2cc161d3f8d1c081f0afd9a8438ab9358cbdbee3 Mon Sep 17 00:00:00 2001 From: spoondla Date: Fri, 23 Jan 2026 17:28:54 -0800 Subject: [PATCH v2] Fix (bug #19382) server crash when ALTER TYPE is used mid-transaction in PL/pgSQL When ALTER TYPE changes a composite type's column types within a transaction, PL/pgSQL record variables that were populated before the ALTER still hold data in the old format. Returning such records causes a crash because the output functions expect data matching the new type definition, not the old one. The crash manifested as a segmentation fault in record_out() when it attempted to interpret integer data as a text pointer, due to the mismatch between the stored data and the current type definition. --- src/pl/plpgsql/src/pl_exec.c | 165 ++++++++++++++++++++++++++++++++++- 1 file changed, 164 insertions(+), 1 deletion(-) diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 6b077febdc8..f2ff1aa25ec 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -458,6 +458,8 @@ static char *format_preparedparamsdata(PLpgSQL_execstate *estate, static PLpgSQL_variable *make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr); +static void convert_record_for_altered_type(PLpgSQL_execstate *estate, + PLpgSQL_rec *rec); /* ---------- * plpgsql_exec_function Called by the call handler for @@ -3244,8 +3246,22 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt) } break; - case PLPGSQL_DTYPE_ROW: case PLPGSQL_DTYPE_REC: + { + PLpgSQL_rec *rec = (PLpgSQL_rec *) retvar; + + /* + * Check if the record's composite type was altered since + * the record was populated. If so, convert the data to + * prevent crashes when outputting the record. + */ + if (rec->rectypeid != RECORDOID && rec->erh != NULL && + !ExpandedRecordIsEmpty(rec->erh)) + convert_record_for_altered_type(estate, rec); + } + /* FALL THROUGH */ + + case PLPGSQL_DTYPE_ROW: { /* exec_eval_datum can handle these cases */ int32 rettypmod; @@ -3390,6 +3406,14 @@ exec_stmt_return_next(PLpgSQL_execstate *estate, TupleDesc rec_tupdesc; TupleConversionMap *tupmap; + /* + * Check if the record's composite type was altered since + * the record was populated. If so, convert the data to + * prevent crashes when storing to the tuplestore. + */ + if (rec->rectypeid != RECORDOID && rec->erh != NULL) + convert_record_for_altered_type(estate, rec); + /* If rec is null, try to convert it to a row of nulls */ if (rec->erh == NULL) instantiate_empty_record_variable(estate, rec); @@ -8883,3 +8907,142 @@ format_preparedparamsdata(PLpgSQL_execstate *estate, return paramstr.data; } + +/* + * convert_record_for_altered_type + * + * Check if a record's composite type has been altered since the record + * was populated, and if so, convert the record data to match the new + * type definition. This prevents crashes that can occur when the stored + * data doesn't match the current type definition. + * + * If conversion is needed, assigns the new record to rec via + * assign_record_var(), which transfers it to datum_context and frees + * the old record. + */ +static void +convert_record_for_altered_type(PLpgSQL_execstate *estate, + PLpgSQL_rec *rec) +{ + ExpandedRecordHeader *erh = rec->erh; + Oid rectypeid = rec->rectypeid; + TupleDesc old_tupdesc; + TupleDesc new_tupdesc; + TypeCacheEntry *typentry; + uint64 current_tupdesc_id; + ExpandedRecordHeader *new_erh; + Datum *old_values; + bool *old_nulls; + Datum *new_values; + bool *new_nulls; + int natts; + int i; + MemoryContext oldcxt; + bool need_conversion = false; + + /* Nothing to do for anonymous RECORD type */ + if (rectypeid == RECORDOID) + return; + + /* Get current type definition from typcache */ + typentry = lookup_type_cache(rectypeid, + TYPECACHE_TUPDESC | + TYPECACHE_DOMAIN_BASE_INFO); + if (typentry->typtype == TYPTYPE_DOMAIN) + typentry = lookup_type_cache(typentry->domainBaseType, + TYPECACHE_TUPDESC); + + current_tupdesc_id = typentry->tupDesc_identifier; + + /* If type hasn't changed, nothing to do (fast path) */ + if (erh->er_tupdesc_id == current_tupdesc_id) + return; + + /* + * Type version has changed. Need to check if field types actually differ + * and convert if necessary. + */ + old_tupdesc = erh->er_tupdesc; + new_tupdesc = typentry->tupDesc; + + /* Sanity check: must have same number of attributes */ + if (old_tupdesc->natts != new_tupdesc->natts) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("record type \"%s\" structure has changed", + format_type_be(rectypeid)), + errdetail("Number of columns changed from %d to %d.", + old_tupdesc->natts, new_tupdesc->natts))); + + natts = old_tupdesc->natts; + + /* Deconstruct the old record to access field values */ + deconstruct_expanded_record(erh); + old_values = erh->dvalues; + old_nulls = erh->dnulls; + + /* Allocate arrays for new values */ + oldcxt = MemoryContextSwitchTo(get_eval_mcontext(estate)); + new_values = (Datum *) palloc(natts * sizeof(Datum)); + new_nulls = (bool *) palloc(natts * sizeof(bool)); + MemoryContextSwitchTo(oldcxt); + + /* Convert each field */ + for (i = 0; i < natts; i++) + { + Form_pg_attribute old_att = TupleDescAttr(old_tupdesc, i); + Form_pg_attribute new_att = TupleDescAttr(new_tupdesc, i); + + /* Skip dropped columns */ + if (old_att->attisdropped || new_att->attisdropped) + { + new_values[i] = (Datum) 0; + new_nulls[i] = true; + continue; + } + + /* If null, stays null */ + if (old_nulls[i]) + { + new_values[i] = (Datum) 0; + new_nulls[i] = true; + continue; + } + + /* If same type, no conversion needed */ + if (old_att->atttypid == new_att->atttypid && + (old_att->atttypmod == new_att->atttypmod || + new_att->atttypmod == -1)) + { + new_values[i] = old_values[i]; + new_nulls[i] = false; + continue; + } + + /* Different type: convert using exec_cast_value */ + need_conversion = true; + new_nulls[i] = false; + new_values[i] = exec_cast_value(estate, + old_values[i], + &new_nulls[i], + old_att->atttypid, + old_att->atttypmod, + new_att->atttypid, + new_att->atttypmod); + } + + /* If no actual conversion was needed, return without modifying rec */ + if (!need_conversion) + return; + + /* Build new expanded record with converted values */ + new_erh = make_expanded_record_from_typeid(rectypeid, -1, + get_eval_mcontext(estate)); + expanded_record_set_fields(new_erh, new_values, new_nulls, true); + + /* + * Assign the new record to rec, transferring it to datum_context + * and freeing the old record. + */ + assign_record_var(estate, rec, new_erh); +} -- 2.39.5 (Apple Git-154)