On Mon, Mar 16, 2026 at 8:39 PM zengman <[email protected]> wrote: > > Hi, > > The commit 2f094e7ac691abc9d2fe0f4dcf0feac4a6ce1d9c delivers an excellent > feature, and the file `rewriteGraphTable.c` was introduced as part of this > commit. > I've noticed that this code file could benefit from some tidying up, so > please review the diff file attached below. > This diff only includes **basic cleanup** of `rewriteGraphTable.c` – it’s > just a simple pass, > and there may be other minor omissions that haven’t been addressed yet.
Hi Man, Thanks for the patch. I reviewed the cleanup you propose. Most of the changes look good to me, except the changes related to pstate. We have both patterns in the code, one which uses free_parsestate() explicitly and another that doesn't. In this case, we don't set p_target_relation, and we don't touch p_next_resno. So the only thing that free_parsestate() does it to free pstate, which will taken care of by the memory context management. So not needed. Any other reason you want to call free_parsestate() explicitly? Attached patch has changes other that pstate related changes. I think, Peter may want to collect more such changes before applying them. -- Best Wishes, Ashutosh Bapat
cleanup_rewrite_graphtable.patch.no_cibot
Description: Binary data
