8000 pageinspect: Fix relcache leak in gist_page_items(). · postgrespro/postgres@04eb75e · GitHub
[go: up one dir, main page]
Skip to content

    • Commit 04eb75e

    Browse files
    hlinnakahlinnaka
    committed
    pageinspect: Fix relcache leak in gist_page_items().
    The gist_page_items() function opened the index relation on first call and closed it on the last call. But there's no guarantee that the function is run to completion, leading to a relcache leak and warning at the end of the transaction. To fix, refactor the function to return all the rows in one call, as a tuplestore. Reported-by: Tom Lane Discussion: https://www.postgresql.org/message-id/234863.1610916631%40sss.pgh.pa.us
    1 parent 7db0cd2 commit 04eb75e

    File tree

    1 file changed

    +74
    -96
    lines changed

    1 file changed

    +74
    -96
    lines changed

    ‎contrib/pageinspect/gistfuncs.c

    Lines changed: 74 additions & 96 deletions
    Original file line numberDiff line numberDiff line change
    @@ -92,65 +92,56 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
    9292
    return HeapTupleGetDatum(resultTuple);
    9393
    }
    9494

    95-
    typedef struct gist_page_items_state
    96-
    {
    97-
    Page page;
    98-
    TupleDesc tupd;
    99-
    OffsetNumber offset;
    100-
    Relation rel;
    101-
    } gist_page_items_state;
    102-
    10395
    Datum
    10496
    gist_page_items_bytea(PG_FUNCTION_ARGS)
    10597
    {
    10698
    bytea *raw_page = PG_GETARG_BYTEA_P(0);
    107-
    FuncCallContext *fctx;
    108-
    gist_page_items_state *inter_call_data;
    99+
    ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
    100+
    bool randomAccess;
    101+
    TupleDesc tupdesc;
    102+
    Tuplestorestate *tupstore;
    103+
    MemoryContext oldcontext;
    104+
    Page page;
    105+
    OffsetNumber offset;
    109106

    110107
    if (!superuser())
    111108
    ereport(ERROR,
    112109
    (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
    113110
    errmsg("must be superuser to use raw page functions")));
    114111

    115-
    if (SRF_IS_FIRSTCALL())
    116-
    {
    117-
    TupleDesc tupdesc;
    118-
    MemoryContext mctx;
    119-
    Page page;
    120-
    121-
    fctx = SRF_FIRSTCALL_INIT();
    122-
    mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
    123-
    124-
    page = get_page_from_raw(raw_page);
    125-
    126-
    inter_call_data = palloc(sizeof(gist_page_items_state));
    112+
    /* check to see if caller supports us returning a tuplestore */
    113+
    if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
    114+
    ereport(ERROR,
    115+
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    116+
    errmsg("set-valued function called in context that cannot accept a set")));
    117+
    if (!(rsinfo->allowedModes & SFRM_Materialize))
    118+
    ereport(ERROR,
    119+
    (errcode(ERRCODE_SYNTAX_ERROR),
    120+
    errmsg("materialize mode required, but it is not allowed in this context")));
    127121

    128-
    /* Build a tuple descriptor for our result type */
    129-
    if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
    130-
    elog(ERROR, "return type must be a row type");
    122+
    /* The tupdesc and tuplestore must be created in ecxt_per_query_memory */
    123+
    oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
    131124

    132-
    if (GistPageIsDeleted(page))
    133-
    elog(NOTICE, "page is deleted");
    125+
    if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
    126+
    elog(ERROR, "return type must be a row type");
    134127

    135-
    inter_call_data->page = page;
    136-
    inter_call_data->tupd = tupdesc;
    137-
    inter_call_data->offset = FirstOffsetNumber;
    128+
    randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
    129+
    tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
    130+
    rsinfo->returnMode = SFRM_Materialize;
    131+
    rsinfo->setResult = tupstore;
    132+
    rsinfo->setDesc = tupdesc;
    138133

    139-
    fctx->max_calls = PageGetMaxOffsetNumber(page);
    140-
    fctx->user_fctx = inter_call_data;
    134+
    MemoryContextSwitchTo(oldcontext);
    141135

    142-
    MemoryContextSwitchTo(mctx);
    143-
    }
    136+
    page = get_page_from_raw(raw_page);
    144137

    145-
    fctx = SRF_PERCALL_SETUP();
    146-
    inter_call_data = fctx->user_fctx;
    138+
    if (GistPageIsDeleted(page))
    139+
    elog(NOTICE, "page is deleted");
    147140

    148-
    if (fctx->call_cntr < fctx->max_calls)
    141+
    for (offset = FirstOffsetNumber;
    142+
    offset <= PageGetMaxOffsetNumber(page);
    143+
    offset++)
    149144
    {
    150-
    Page page = inter_call_data->page;
    151-
    OffsetNumber offset = inter_call_data->offset;
    152-
    HeapTuple resultTuple;
    153-
    Datum result;
    154145
    Datum values[4];
    155146
    bool nulls[4];
    156147
    ItemId id;
    @@ -177,74 +168,67 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
    177168
    memcpy(VARDATA(tuple_bytea), itup, tuple_len);
    178169
    values[3] = PointerGetDatum(tuple_bytea);
    179170

    180-
    /* Build and return the result tuple. */
    181-
    resultTuple = heap_form_tuple(inter_call_data->tupd, values, nulls);
    182-
    result = HeapTupleGetDatum(resultTuple);
    183-
    184-
    inter_call_data->offset++;
    185-
    SRF_RETURN_NEXT(fctx, result);
    171+
    tuplestore_putvalues(tupstore, tupdesc, values, nulls);
    186172
    }
    187173

    188-
    SRF_RETURN_DONE(fctx);
    174+
    return (Datum) 0;
    189175
    }
    190176

    191177
    Datum
    192178
    gist_page_items(PG_FUNCTION_ARGS)
    193179
    {
    194180
    bytea *raw_page = PG_GETARG_BYTEA_P(0);
    195181
    Oid indexRelid = PG_GETARG_OID(1);
    196-
    FuncCallContext *fctx;
    197-
    gist_page_items_state *inter_call_data;
    182+
    ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
    183+
    bool randomAccess;
    184+
    Relation indexRel;
    185+
    TupleDesc tupdesc;
    186+
    Tuplestorestate *tupstore;
    187+
    MemoryContext oldcontext;
    188+
    Page page;
    189+
    OffsetNumber offset;
    198190

    199191
    if (!superuser())
    200192
    ereport(ERROR,
    201193
    (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
    202194
    errmsg("must be superuser to use raw page functions")));
    203195

    204-
    if (SRF_IS_FIRSTCALL())
    205-
    {
    206-
    Relation indexRel;
    207-
    TupleDesc tupdesc;
    208-
    MemoryContext mctx;
    209-
    Page page;
    210-
    211-
    fctx = SRF_FIRSTCALL_INIT();
    212-
    mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
    213-
    214-
    page = get_page_from_raw(raw_page);
    215-
    216-
    inter_call_data = palloc(sizeof(gist_page_items_state));
    196+
    /* check to see if caller supports us returning a tuplestore */
    197+
    if< 10000 /span> (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
    198+
    ereport(ERROR,
    199+
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    200+
    errmsg("set-valued function called in context that cannot accept a set")));
    201+
    if (!(rsinfo->allowedModes & SFRM_Materialize))
    202+
    ereport(ERROR,
    203+
    (errcode(ERRCODE_SYNTAX_ERROR),
    204+
    errmsg("materialize mode required, but it is not allowed in this context")));
    217205

    218-
    /* Open the relation */
    219-
    indexRel = index_open(indexRelid, AccessShareLock);
    206+
    /* The tupdesc and tuplestore must be created in ecxt_per_query_memory */
    207+
    oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
    220208

    221-
    /* Build a tuple descriptor for our result type */
    222-
    if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
    223-
    elog(ERROR, "return type must be a row type");
    209+
    if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
    210+
    elog(ERROR, "return type must be a row type");
    224211

    225-
    if (GistPageIsDeleted(page))
    226-
    elog(NOTICE, "page is deleted");
    212+
    randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
    213+
    tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
    214+
    rsinfo->returnMode = SFRM_Materialize;
    215+
    rsinfo->setResult = tupstore;
    216+
    rsinfo->setDesc = tupdesc;
    227217

    228-
    inter_call_data->page = page;
    229-
    inter_call_data->tupd = tupdesc;
    230-
    inter_call_data->offset = FirstOffsetNumber;
    231-
    inter_call_data->rel = indexRel;
    218+
    MemoryContextSwitchTo(oldcontext);
    232219

    233-
    fctx->max_calls = PageGetMaxOffsetNumber(page);
    234-
    fctx->user_fctx = inter_call_data;
    220+
    /* Open the relation */
    221+
    indexRel = index_open(indexRelid, AccessShareLock);
    235222

    236-
    MemoryContextSwitchTo(mctx);
    237-
    }
    223+
    page = get_page_from_raw(raw_page);
    238224

    239-
    fctx = SRF_PERCALL_SETUP();
    240-
    inter_call_data = fctx->user_fctx;
    225+
    if (GistPageIsDeleted(page))
    226+
    elog(NOTICE, "page is deleted");
    241227

    242-
    if (fctx->call_cntr < fctx->max_calls)
    228+
    for (offset = FirstOffsetNumber;
    229+
    offset <= PageGetMaxOffsetNumber(page);
    230+
    offset++)
    243231
    {
    244-
    Page page = inter_call_data->page;
    245-
    OffsetNumber offset = inter_call_data->offset;
    246-
    HeapTuple resultTuple;
    247-
    Datum result;
    248232
    Datum values[4];
    249233
    bool nulls[4];
    250234
    ItemId id;
    @@ -260,11 +244,10 @@ gist_page_items(PG_FUNCTION_ARGS)
    260244

    261245
    itup = (IndexTuple) PageGetItem(page, id);
    262246

    263-
    index_deform_tuple(itup, RelationGetDescr(inter_call_data->rel),
    247+
    index_deform_tuple(itup, RelationGetDescr(indexRel),
    264248
    itup_values, itup_isnull);
    265249

    266-
    key_desc = BuildIndexValueDescription(inter_call_data->rel, itup_values,
    267-
    itup_isnull);
    250+
    key_desc = BuildIndexValueDescription(indexRel, itup_values, itup_isnull);
    268251

    269252
    memset(nulls, 0, sizeof(nulls));
    270253

    @@ -273,15 +256,10 @@ gist_page_items(PG_FUNCTION_ARGS)
    273256
    values[2] = Int32GetDatum((int) IndexTupleSize(itup));
    274257
    values[3] = CStringGetTextDatum(key_desc);
    275258

    276-
    /* Build and return the result tuple. */
    277-
    resultTuple = heap_form_tuple(inter_call_data->tupd, values, nulls);
    278-
    result = HeapTupleGetDatum(resultTuple);
    279-
    280-
    inter_call_data->offset++;
    281-
    SRF_RETURN_NEXT(fctx, result);
    259+
    tuplestore_putvalues(tupstore, tupdesc, values, nulls);
    282260
    }
    283261

    284-
    relation_close(inter_call_data->rel, AccessShareLock);
    262+
    relation_close(indexRel, AccessShareLock);
    285263

    286-
    SRF_RETURN_DONE(fctx);
    264+
    return (Datum) 0;
    287265
    }

    0 commit comments

    Comments
     (0)
    0