Thread: Fix comments of heap_prune_chain()
Hi,
While I’m reading source codes related to vacuum, I found comments which
don’t seem to fit the reality. I think the commit[1] just forgot to fix them.
What do you think?
[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=dc7420c2c9274a283779ec19718d2d16323640c0
While I’m reading source codes related to vacuum, I found comments which
don’t seem to fit the reality. I think the commit[1] just forgot to fix them.
What do you think?
[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=dc7420c2c9274a283779ec19718d2d16323640c0
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
Masahiro Ikeda
NTT DATA CORPORATION
Attachment
On 2021-Jul-12, ikedamsh@oss.nttdata.com wrote: > While I’m reading source codes related to vacuum, I found comments which > don’t seem to fit the reality. I think the commit[1] just forgot to fix them. > What do you think? > > [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=dc7420c2c9274a283779ec19718d2d16323640c0 That sounds believable, but can you be more specific? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
On Mon, 12 Jul 2021 at 13:14, <ikedamsh@oss.nttdata.com> wrote:
>
> Hi,
>
> While I’m reading source codes related to vacuum, I found comments which
> don’t seem to fit the reality. I think the commit[1] just forgot to fix them.
> What do you think?
Hmm, yes, those are indeed some leftovers.
Some comments on the suggested changes:
- * caused by HeapTupleSatisfiesVacuum. We just add entries to the arrays in
+ * caused by heap_prune_satisfies_vacuum. We just add entries to the arrays in
I think that HeapTupleSatisfiesVacuumHorizon might be an alternative correct replacement here.
- elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ elog(ERROR, "unexpected heap_prune_satisfies_vacuum result");
The type of the value is HTSV_Result; where HTSV stands for HeapTupleSatisfiesVacuum, so if we were to replace this, I'd go for "unexpected result from heap_prune_satisfies_vacuum" as a message instead.
Kind regards,
Matthias van de Meent
On 2021-Jul-12, Alvaro Herrera wrote: > On 2021-Jul-12, ikedamsh@oss.nttdata.com wrote: > > > While I’m reading source codes related to vacuum, I found comments which > > don’t seem to fit the reality. I think the commit[1] just forgot to fix them. > > What do you think? > > > > [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=dc7420c2c9274a283779ec19718d2d16323640c0 > > That sounds believable, but can you be more specific? Oh, apologies, I didn't realize there was an attachment. That seems specific enough :-) In my defense, the archives don't show the attachment either: https://www.postgresql.org/message-id/5CB29811-2B1D-4244-8DE2-B1E02495426B%40oss.nttdata.com I think we've seen this kind of problem before -- the MIME structure of the message is quite unusual, which is why neither my MUA nor the archives show it. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Los trabajadores menos efectivos son sistematicamente llevados al lugar donde pueden hacer el menor daño posible: gerencia." (El principio Dilbert)
(This is out of topic) At Mon, 12 Jul 2021 20:17:55 -0400, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in > Oh, apologies, I didn't realize there was an attachment. That seems > specific enough :-) > > In my defense, the archives don't show the attachment either: > https://www.postgresql.org/message-id/5CB29811-2B1D-4244-8DE2-B1E02495426B%40oss.nttdata.com > I think we've seen this kind of problem before -- the MIME structure of > the message is quite unusual, which is why neither my MUA nor the > archives show it. The same for me. Multipart structure of that mail looks like odd. multipart/laternative text/plain - mail body quoted-printable multipart/mixed text/html - HTML alternative body appliation/octet-stream - the patch text/html - garbage I found an issue in bugzilla about this behavior https://bugzilla.mozilla.org/show_bug.cgi?id=1362539 The primary issue is Apple-mail's strange mime-composition. I'm not sure whether it is avoidable by some settings. (I don't think the alternative HTML body is useful at least for this mailling list.) regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On 2021/07/13 5:57, Matthias van de Meent wrote: > > > On Mon, 12 Jul 2021 at 13:14, <ikedamsh@oss.nttdata.com > <mailto:ikedamsh@oss.nttdata.com>> wrote: >> >> Hi, >> >> While I’m reading source codes related to vacuum, I found comments which >> don’t seem to fit the reality. I think the commit[1] just forgot to fix them. >> What do you think? > > Hmm, yes, those are indeed some leftovers. > > Some comments on the suggested changes: > > > - * caused by HeapTupleSatisfiesVacuum. We just add entries to the arrays in > + * caused by heap_prune_satisfies_vacuum. We just add entries to the arrays in > > I think that HeapTupleSatisfiesVacuumHorizon might be an alternative correct > replacement here. > > > - elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result"); > + elog(ERROR, "unexpected heap_prune_satisfies_vacuum result"); > > The type of the value is HTSV_Result; where HTSV stands for > HeapTupleSatisfiesVacuum, so if we were to replace this, I'd go for > "unexpected result from heap_prune_satisfies_vacuum" as a message instead. Thanks for your comments. I agree your suggestions. I also updated prstate->vistest to heap_prune_satisfies_vacuum of v1 patch because heap_prune_satisfies_vacuum() tests with not only prstate->vistest but also prstate->old_snap_xmin. I think it's more accurate representation. Regards, -- Masahiro Ikeda NTT DATA CORPORATION
Attachment
On 2021/07/13 10:22, Kyotaro Horiguchi wrote: > (This is out of topic) > > At Mon, 12 Jul 2021 20:17:55 -0400, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in >> Oh, apologies, I didn't realize there was an attachment. That seems >> specific enough :-) >> >> In my defense, the archives don't show the attachment either: >> https://www.postgresql.org/message-id/5CB29811-2B1D-4244-8DE2-B1E02495426B%40oss.nttdata.com >> I think we've seen this kind of problem before -- the MIME structure of >> the message is quite unusual, which is why neither my MUA nor the >> archives show it. > > The same for me. Multipart structure of that mail looks like odd. > > multipart/laternative > text/plain - mail body quoted-printable > multipart/mixed > text/html - HTML alternative body > appliation/octet-stream - the patch > text/html - garbage > > > I found an issue in bugzilla about this behavior > > https://bugzilla.mozilla.org/show_bug.cgi?id=1362539 > > The primary issue is Apple-mail's strange mime-composition. > I'm not sure whether it is avoidable by some settings. > > (I don't think the alternative HTML body is useful at least for this > mailling list.) Thanks for replying and sorry for the above. The reason is that I sent from MacBook PC as Horiguchi-san said. I changed my email client and I confirmed that I could send an email with a new patch. So, please check it. https://www.postgresql.org/message-id/1aa07e2a-b715-5649-6c62-4fff96304d18%40oss.nttdata.com Regards, -- Masahiro Ikeda NTT DATA CORPORATION