On 10/31/21, 1:55 PM, "Tom Lane" <t...@sss.pgh.pa.us> wrote: > 1. Attached is a proposed patch to get rid of the calls in trgm_regexp.c. > I'm not certain that those lists could get long enough to be a problem, > given the existing complexity limits in that file (MAX_EXPANDED_STATES > etc). But I'm not certain they can't, either, and it's easy enough to > fix along the same lines as in StandbyReleaseLockList.
Should there be a list_free(trgmNFA->queue) at the end of transformGraph()? > 2. I think we almost certainly have a problem in SyncPostCheckpoint. This one doesn't look as straightforward. It looks like we might need a list_delete_first_n() to delete the first N entries all at once to improve this one. > 3. Is agg_refill_hash_table a problem? Probably; we've seen cases > with lots of batches. IIUC this one can be improved by pushing/popping from the end of the list instead of the beginning. > 4. I'm a bit worried about the uses in access/gist/, but I don't know > that code well enough to want to mess with it. It's possible the > list lengths are bounded by the index tree height, in which case it > likely doesn't matter. The logic in gistFindPath looks like a mess > anyway since it's appending to both ends of the "fifo" list in different > places (is that really necessary?). > > 5. Not sure about CopyMultiInsertInfoFlush ... how many buffers > could we have there? I haven't looked too closely at these. > 6. llvm_release_context may not have a long enough list to be a > problem, but on the other hand, it looks easy to fix. Yeah, I reviewed this one earlier. I didn't see any reason this one couldn't be changed to foreach(). > 7. The list lengths in the parser and dependency.c, ruleutils.c, > explain.c are bounded by subquery nesting depth or plan tree depth, > so I doubt it's worth worrying about. +1 > 8. The uses in namespace.c don't seem like an issue either -- for > instance, GetOverrideSearchPath can't iterate more than twice, > and the overrideStack list shouldn't get very deep. +1 Nathan