"Bossart, Nathan" <bossa...@amazon.com> writes:
> Ah, I see it now. The patch looks good to me, then.
Thanks for looking! Here's an expanded patch that also takes care
of the other two easy-to-fix cases, nodeAgg.c and llvmjit.c.
AFAICS, llvm_release_context is like StandbyReleaseLockList
in that we don't need to worry about whether the data structure
is valid after an error partway through. (Maybe we should be
worrying, but I think the callers would need work as well if
that's to be the standard.)
regards, tom lane
diff --git a/contrib/pg_trgm/trgm_regexp.c b/contrib/pg_trgm/trgm_regexp.c
index 64816dd370..1887a39161 100644
--- a/contrib/pg_trgm/trgm_regexp.c
+++ b/contrib/pg_trgm/trgm_regexp.c
@@ -907,6 +907,7 @@ transformGraph(TrgmNFA *trgmNFA)
HASHCTL hashCtl;
TrgmStateKey initkey;
TrgmState *initstate;
+ ListCell *lc;
/* Initialize this stage's workspace in trgmNFA struct */
trgmNFA->queue = NIL;
@@ -937,12 +938,13 @@ transformGraph(TrgmNFA *trgmNFA)
/*
* Recursively build the expanded graph by processing queue of states
* (breadth-first search). getState already put initstate in the queue.
+ * Note that getState will append new states to the queue within the loop,
+ * too; this works as long as we don't do repeat fetches using the "lc"
+ * pointer.
*/
- while (trgmNFA->queue != NIL)
+ foreach(lc, trgmNFA->queue)
{
- TrgmState *state = (TrgmState *) linitial(trgmNFA->queue);
-
- trgmNFA->queue = list_delete_first(trgmNFA->queue);
+ TrgmState *state = (TrgmState *) lfirst(lc);
/*
* If we overflowed then just mark state as final. Otherwise do
@@ -966,22 +968,29 @@ transformGraph(TrgmNFA *trgmNFA)
static void
processState(TrgmNFA *trgmNFA, TrgmState *state)
{
+ ListCell *lc;
+
/* keysQueue should be NIL already, but make sure */
trgmNFA->keysQueue = NIL;
/*
* Add state's own key, and then process all keys added to keysQueue until
- * queue is empty. But we can quit if the state gets marked final.
+ * queue is finished. But we can quit if the state gets marked final.
*/
addKey(trgmNFA, state, &state->stateKey);
- while (trgmNFA->keysQueue != NIL && !(state->flags & TSTATE_FIN))
+ foreach(lc, trgmNFA->keysQueue)
{
- TrgmStateKey *key = (TrgmStateKey *) linitial(trgmNFA->keysQueue);
+ TrgmStateKey *key = (TrgmStateKey *) lfirst(lc);
- trgmNFA->keysQueue = list_delete_first(trgmNFA->keysQueue);
+ if (state->flags & TSTATE_FIN)
+ break;
addKey(trgmNFA, state, key);
}
+ /* Release keysQueue to clean up for next cycle */
+ list_free(trgmNFA->keysQueue);
+ trgmNFA->keysQueue = NIL;
+
/*
* Add outgoing arcs only if state isn't final (we have no interest in
* outgoing arcs if we already match)
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index c99a0de4dd..f5a187cae3 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -2584,8 +2584,9 @@ agg_refill_hash_table(AggState *aggstate)
if (aggstate->hash_batches == NIL)
return false;
- batch = linitial(aggstate->hash_batches);
- aggstate->hash_batches = list_delete_first(aggstate->hash_batches);
+ /* hash_batches is a stack, with the top item at the end of the list */
+ batch = llast(aggstate->hash_batches);
+ aggstate->hash_batches = list_delete_last(aggstate->hash_batches);
hash_agg_set_limits(aggstate->hashentrysize, batch->input_card,
batch->used_bits, &aggstate->hash_mem_limit,
@@ -3098,7 +3099,7 @@ hashagg_spill_finish(AggState *aggstate, HashAggSpill *spill, int setno)
new_batch = hashagg_batch_new(tape, setno,
spill->ntuples[i], cardinality,
used_bits);
- aggstate->hash_batches = lcons(new_batch, aggstate->hash_batches);
+ aggstate->hash_batches = lappend(aggstate->hash_batches, new_batch);
aggstate->hash_batches_used++;
}
@@ -3113,8 +3114,6 @@ hashagg_spill_finish(AggState *aggstate, HashAggSpill *spill, int setno)
static void
hashagg_reset_spill_state(AggState *aggstate)
{
- ListCell *lc;
-
/* free spills from initial pass */
if (aggstate->hash_spills != NULL)
{
@@ -3132,13 +3131,7 @@ hashagg_reset_spill_state(AggState *aggstate)
}
/* free batches */
- foreach(lc, aggstate->hash_batches)
- {
- HashAggBatch *batch = (HashAggBatch *) lfirst(lc);
-
- pfree(batch);
- }
- list_free(aggstate->hash_batches);
+ list_free_deep(aggstate->hash_batches);
aggstate->hash_batches = NIL;
/* close tape set */
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 169dad96d7..fb29449573 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -171,6 +171,7 @@ static void
llvm_release_context(JitContext *context)
{
LLVMJitContext *llvm_context = (LLVMJitContext *) context;
+ ListCell *lc;
/*
* When this backend is exiting, don't clean up LLVM. As an error might
@@ -188,12 +189,9 @@ llvm_release_context(JitContext *context)
llvm_context->module = NULL;
}
- while (llvm_context->handles != NIL)
+ foreach(lc, llvm_context->handles)
{
- LLVMJitHandle *jit_handle;
-
- jit_handle = (LLVMJitHandle *) linitial(llvm_context->handles);
- llvm_context->handles = list_delete_first(llvm_context->handles);
+ LLVMJitHandle *jit_handle = (LLVMJitHandle *) lfirst(lc);
#if LLVM_VERSION_MAJOR > 11
{
@@ -221,6 +219,8 @@ llvm_release_context(JitContext *context)
pfree(jit_handle);
}
+ list_free(llvm_context->handles);
+ llvm_context->handles = NIL;
}
/*
