| From: | zengman <zengman(at)halodbtech(dot)com> |
|---|---|
| To: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
| Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: SQL Property Graph Queries (SQL/PGQ) |
| Date: | 2026-03-17 01:26:19 |
| Message-ID: | tencent_5373E3391DFB49CD6609B281@qq.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> > 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.
Hi,
Thank you for your review. I understand your feedback – this was a result of my personal coding habit: I prefer to explicitly call `free_parsestate()`, and also want to respect the comment associated with `make_parsestate()` (shown below):
```
/*
* make_parsestate
* Allocate and initialize a new ParseState.
*
* Caller should eventually release the ParseState via free_parsestate().
*/
```
For this reason, I have made minor modifications to this section of the code.
--
regards,
Man Zeng
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Smith | 2026-03-17 01:52:23 | Re: Skipping schema changes in publication |
| Previous Message | Xuneng Zhou | 2026-03-17 01:15:22 | Re: BUG: Cascading standby fails to reconnect after falling back to archive recovery |