Thread: Incorrect comment regarding command completion tags
I was looking at the code in EndCommand() and noticed a comment talking about some Asserts which didn't seem to exist in the code. The comment also talks about LastOid which looks like the name of a variable that's nowhere to be seen. It looks like the Asserts did exists when the completion tag patch was being developed [1] but they disappeared somewhere along the line and the comment didn't get an update before 2f9661311 went in. In the attached, I rewrote the comment to remove mention of the Asserts. I also tried to form the comment in a way that's more understandable about why we always write a "0" in "INSERT 0 <nrows>". David [1] https://www.postgresql.org/message-id/1C642743-8E46-4246-B4A0-C9A638B3E88F@enterprisedb.com
Attachment
> On Oct 13, 2022, at 2:56 PM, David Rowley <dgrowleyml@gmail.com> wrote: > > I was looking at the code in EndCommand() and noticed a comment > talking about some Asserts which didn't seem to exist in the code. > The comment also talks about LastOid which looks like the name of a > variable that's nowhere to be seen. > > It looks like the Asserts did exists when the completion tag patch was > being developed [1] but they disappeared somewhere along the line and > the comment didn't get an update before 2f9661311 went in. > > In the attached, I rewrote the comment to remove mention of the > Asserts. I also tried to form the comment in a way that's more > understandable about why we always write a "0" in "INSERT 0 <nrows>". Your wording is better. +1 — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, 14 Oct 2022 at 11:38, Mark Dilger <mark.dilger@enterprisedb.com> wrote: > > > On Oct 13, 2022, at 2:56 PM, David Rowley <dgrowleyml@gmail.com> wrote: > > In the attached, I rewrote the comment to remove mention of the > > Asserts. I also tried to form the comment in a way that's more > > understandable about why we always write a "0" in "INSERT 0 <nrows>". > > Your wording is better. +1 Thanks for having a look. I adjusted the wording slightly as I had written "ancient" in regards to PostgreSQL 11 and earlier. It's probably a bit early to call a supported version of PostgreSQL ancient so I just decided to mention the version number instead. I pushed the resulting patch. Thanks for having a look. David