Thread
Commits
-
Improve comments referring snapshot's subxip array.
- 8b5262fa0efd 16.0 landed
-
Typo about subxip in comments
Japin Li <japinli@hotmail.com> — 2022-11-11T03:26:13Z
Hi, hackers Recently, when I read the XidInMVCCSnapshot(), and find there are some typos in the comments. diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 207c4b27fd..9e8b6756fe 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2409,7 +2409,7 @@ GetSnapshotData(Snapshot snapshot) * We could try to store xids into xip[] first and then into subxip[] * if there are too many xids. That only works if the snapshot doesn't * overflow because we do not search subxip[] in that case. A simpler - * way is to just store all xids in the subxact array because this is + * way is to just store all xids in the subxip array because this is * by far the bigger array. We just leave the xip array empty. * * Either way we need to change the way XidInMVCCSnapshot() works diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index f1f2ddac17..2524b1c585 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -2345,7 +2345,7 @@ XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot) else { /* - * In recovery we store all xids in the subxact array because it is by + * In recovery we store all xids in the subxip array because it is by * far the bigger array, and we mostly don't know which xids are * top-level and which are subxacts. The xip array is empty. * -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd. -
Re: Typo about subxip in comments
Amit Kapila <amit.kapila16@gmail.com> — 2022-11-11T05:09:06Z
On Fri, Nov 11, 2022 at 8:56 AM Japin Li <japinli@hotmail.com> wrote: > > Recently, when I read the XidInMVCCSnapshot(), and find there are some > typos in the comments. > > diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c > index 207c4b27fd..9e8b6756fe 100644 > --- a/src/backend/storage/ipc/procarray.c > +++ b/src/backend/storage/ipc/procarray.c > @@ -2409,7 +2409,7 @@ GetSnapshotData(Snapshot snapshot) > * We could try to store xids into xip[] first and then into subxip[] > * if there are too many xids. That only works if the snapshot doesn't > * overflow because we do not search subxip[] in that case. A simpler > - * way is to just store all xids in the subxact array because this is > + * way is to just store all xids in the subxip array because this is > * by far the bigger array. We just leave the xip array empty. > * > * Either way we need to change the way XidInMVCCSnapshot() works > diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c > index f1f2ddac17..2524b1c585 100644 > --- a/src/backend/utils/time/snapmgr.c > +++ b/src/backend/utils/time/snapmgr.c > @@ -2345,7 +2345,7 @@ XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot) > else > { > /* > - * In recovery we store all xids in the subxact array because it is by > + * In recovery we store all xids in the subxip array because it is by > * far the bigger array, and we mostly don't know which xids are > * top-level and which are subxacts. The xip array is empty. > * > LGTM. -- With Regards, Amit Kapila. -
Re: Typo about subxip in comments
Julien Rouhaud <rjuju123@gmail.com> — 2022-11-11T05:33:13Z
On Fri, Nov 11, 2022 at 10:39:06AM +0530, Amit Kapila wrote: > On Fri, Nov 11, 2022 at 8:56 AM Japin Li <japinli@hotmail.com> wrote: > > > > Recently, when I read the XidInMVCCSnapshot(), and find there are some > > typos in the comments. > > > > diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c > > index 207c4b27fd..9e8b6756fe 100644 > > --- a/src/backend/storage/ipc/procarray.c > > +++ b/src/backend/storage/ipc/procarray.c > > @@ -2409,7 +2409,7 @@ GetSnapshotData(Snapshot snapshot) > > * We could try to store xids into xip[] first and then into subxip[] > > * if there are too many xids. That only works if the snapshot doesn't > > * overflow because we do not search subxip[] in that case. A simpler > > - * way is to just store all xids in the subxact array because this is > > + * way is to just store all xids in the subxip array because this is > > * by far the bigger array. We just leave the xip array empty. > > * > > * Either way we need to change the way XidInMVCCSnapshot() works > > diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c > > index f1f2ddac17..2524b1c585 100644 > > --- a/src/backend/utils/time/snapmgr.c > > +++ b/src/backend/utils/time/snapmgr.c > > @@ -2345,7 +2345,7 @@ XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot) > > else > > { > > /* > > - * In recovery we store all xids in the subxact array because it is by > > + * In recovery we store all xids in the subxip array because it is by > > * far the bigger array, and we mostly don't know which xids are > > * top-level and which are subxacts. The xip array is empty. > > * > > > > LGTM. +1 -
Re: Typo about subxip in comments
Richard Guo <guofenglinux@gmail.com> — 2022-11-11T06:46:13Z
On Fri, Nov 11, 2022 at 11:26 AM Japin Li <japinli@hotmail.com> wrote: > Recently, when I read the XidInMVCCSnapshot(), and find there are some > typos in the comments. Hmm, it seems to me 'the subxact array' is just another saying to refer to snapshot->subxip. I'm not sure about this being typo. But I have no objection to this change, as it is more consistent with the 'xip array' saying followed. Thanks Richard
-
Re: Typo about subxip in comments
Amit Kapila <amit.kapila16@gmail.com> — 2022-11-11T07:23:15Z
On Fri, Nov 11, 2022 at 12:16 PM Richard Guo <guofenglinux@gmail.com> wrote: > > On Fri, Nov 11, 2022 at 11:26 AM Japin Li <japinli@hotmail.com> wrote: >> >> Recently, when I read the XidInMVCCSnapshot(), and find there are some >> typos in the comments. > > > Hmm, it seems to me 'the subxact array' is just another saying to refer > to snapshot->subxip. I'm not sure about this being typo. But I have no > objection to this change, as it is more consistent with the 'xip array' > saying followed. > Agreed, it is more about being consistent with xip array. -- With Regards, Amit Kapila.
-
Re: Typo about subxip in comments
Japin Li <japinli@hotmail.com> — 2022-11-11T09:14:52Z
On Fri, 11 Nov 2022 at 15:23, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Fri, Nov 11, 2022 at 12:16 PM Richard Guo <guofenglinux@gmail.com> wrote: >> >> On Fri, Nov 11, 2022 at 11:26 AM Japin Li <japinli@hotmail.com> wrote: >>> >>> Recently, when I read the XidInMVCCSnapshot(), and find there are some >>> typos in the comments. >> >> >> Hmm, it seems to me 'the subxact array' is just another saying to refer >> to snapshot->subxip. I'm not sure about this being typo. But I have no >> objection to this change, as it is more consistent with the 'xip array' >> saying followed. >> > > Agreed, it is more about being consistent with xip array. Thanks for reviewings. Maybe a wrong plural in XidInMvccSnapshot(). * Make a quick range check to eliminate most XIDs without looking at the * xip arrays. I think we should use "xip array" instead of "xip arrays". Furthermore, if the snapshot is taken during recovery, the xip array is empty, and we should check subxip array. How about changing "xip arrays" to "xip or subxip array"? -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd. -
Re: Typo about subxip in comments
Amit Kapila <amit.kapila16@gmail.com> — 2022-11-12T04:12:06Z
On Fri, Nov 11, 2022 at 2:45 PM Japin Li <japinli@hotmail.com> wrote: > > On Fri, 11 Nov 2022 at 15:23, Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Fri, Nov 11, 2022 at 12:16 PM Richard Guo <guofenglinux@gmail.com> wrote: > >> > >> On Fri, Nov 11, 2022 at 11:26 AM Japin Li <japinli@hotmail.com> wrote: > >>> > >>> Recently, when I read the XidInMVCCSnapshot(), and find there are some > >>> typos in the comments. > >> > >> > >> Hmm, it seems to me 'the subxact array' is just another saying to refer > >> to snapshot->subxip. I'm not sure about this being typo. But I have no > >> objection to this change, as it is more consistent with the 'xip array' > >> saying followed. > >> > > > > Agreed, it is more about being consistent with xip array. > > Thanks for reviewings. > > Maybe a wrong plural in XidInMvccSnapshot(). > > * Make a quick range check to eliminate most XIDs without looking at the > * xip arrays. > > I think we should use "xip array" instead of "xip arrays". > I think here the comment is referring to both xip and subxip array, so it looks okay to me. > Furthermore, if the snapshot is taken during recovery, the xip array is > empty, and we should check subxip array. How about changing "xip arrays" > to "xip or subxip array"? > I don't know if that is an improvement. I think we should stick to your initial proposed change. -- With Regards, Amit Kapila.
-
Re: Typo about subxip in comments
Japin Li <japinli@hotmail.com> — 2022-11-13T15:02:06Z
On Sat, 12 Nov 2022 at 12:12, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Fri, Nov 11, 2022 at 2:45 PM Japin Li <japinli@hotmail.com> wrote: >> Maybe a wrong plural in XidInMvccSnapshot(). >> >> * Make a quick range check to eliminate most XIDs without looking at the >> * xip arrays. >> >> I think we should use "xip array" instead of "xip arrays". >> > > I think here the comment is referring to both xip and subxip array, so > it looks okay to me. > Yeah, it means xip in normal case, and subxip in recovery case. >> Furthermore, if the snapshot is taken during recovery, the xip array is >> empty, and we should check subxip array. How about changing "xip arrays" >> to "xip or subxip array"? >> > > I don't know if that is an improvement. I think we should stick to > your initial proposed change. Agreed. Let's focus on the initial proposed change. -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.
-
Re: Typo about subxip in comments
Amit Kapila <amit.kapila16@gmail.com> — 2022-11-15T07:05:34Z
On Sun, Nov 13, 2022 at 8:32 PM Japin Li <japinli@hotmail.com> wrote: > > On Sat, 12 Nov 2022 at 12:12, Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > I don't know if that is an improvement. I think we should stick to > > your initial proposed change. > > Agreed. Let's focus on the initial proposed change. > Pushed. -- With Regards, Amit Kapila.