Obsolete comment in commands/analyze.c

Started by Etsuro Fujitaover 6 years ago4 messages
#1Etsuro Fujita
etsuro.fujita@gmail.com
1 attachment(s)

Hi,

I think commit 83e176ec1, which moved block sampling functions to
utils/misc/sampling.c, forgot to update this comment in
commands/analyze.c: "This algorithm is from Jeff Vitter's paper (see
full citation below)"; since the citation was also moved to
utils/misc/sampling.c, I think the "see full citation below" part
should be updated accordingly. Attached is a patch for that.

Best regards,
Etsuro Fujita

Attachments:

update-comment.patchapplication/octet-stream; name=update-comment.patchDownload
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 6cb545c126..d7004e5313 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1048,13 +1048,13 @@ acquire_sample_rows(Relation onerel, int elevel,
 			 * The first targrows sample rows are simply copied into the
 			 * reservoir. Then we start replacing tuples in the sample until
 			 * we reach the end of the relation.  This algorithm is from Jeff
-			 * Vitter's paper (see full citation below). It works by
-			 * repeatedly computing the number of tuples to skip before
-			 * selecting a tuple, which replaces a randomly chosen element of
-			 * the reservoir (current set of tuples).  At all times the
-			 * reservoir is a true random sample of the tuples we've passed
-			 * over so far, so when we fall off the end of the relation we're
-			 * done.
+			 * Vitter's paper (see full citation in utils/misc/sampling.c). It
+			 * works by repeatedly computing the number of tuples to skip
+			 * before selecting a tuple, which replaces a randomly chosen
+			 * element of the reservoir (current set of tuples).  At all times
+			 * the reservoir is a true random sample of the tuples we've
+			 * passed over so far, so when we fall off the end of the relation
+			 * we're done.
 			 */
 			if (numrows < targrows)
 				rows[numrows++] = ExecCopySlotHeapTuple(slot);
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Etsuro Fujita (#1)
Re: Obsolete comment in commands/analyze.c

On 27 Jun 2019, at 13:05, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

since the citation was also moved to
utils/misc/sampling.c, I think the "see full citation below" part
should be updated accordingly. Attached is a patch for that.

Agreed, nice catch!

cheers ./daniel

#3Tomas Vondra
tomas.vondra@2ndquadrant.com
In reply to: Daniel Gustafsson (#2)
Re: Obsolete comment in commands/analyze.c

On Thu, Jun 27, 2019 at 01:53:52PM +0200, Daniel Gustafsson wrote:

On 27 Jun 2019, at 13:05, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

since the citation was also moved to
utils/misc/sampling.c, I think the "see full citation below" part
should be updated accordingly. Attached is a patch for that.

Agreed, nice catch!

Thanks, committed.

cheers

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Etsuro Fujita
etsuro.fujita@gmail.com
In reply to: Tomas Vondra (#3)
Re: Obsolete comment in commands/analyze.c

On Fri, Jun 28, 2019 at 1:02 AM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:

On Thu, Jun 27, 2019 at 01:53:52PM +0200, Daniel Gustafsson wrote:

On 27 Jun 2019, at 13:05, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

since the citation was also moved to
utils/misc/sampling.c, I think the "see full citation below" part
should be updated accordingly. Attached is a patch for that.

Agreed, nice catch!

Thanks, committed.

Thanks for committing, Tomas! Thanks for reviewing, Daniel!

Best regards,
Etsuro Fujita