Re: ANALYZE ONLY

From: Michael Harris <harmic(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, postgres(at)jeltef(dot)nl, ilya(dot)evdokimov(at)tantorlabs(dot)com
Subject: Re: ANALYZE ONLY
Date: 2024-09-09 07:56:54
Message-ID: CADofcAVYsvj=RYdBcpqreC-4yc474n-3noyuqzzmq9VagTWS2A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the feedback David.

On Mon, 9 Sept 2024 at 11:27, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> You've written "was" (past tense), but then the existing text uses
> "will" (future tense). I guess if the point in time is after parse and
> before work has been done, then that's correct, but I think using "is"
> instead of "was" is better.

> Maybe "are also vacuumed" instead of "are vacuumed" is more clear?

Agreed. I have updated my patch with both of these suggestions.

> 4. A very minor detail, but I think in vacuum.c the WARNING you've
> added should use RelationGetRelationName(). We seem to be very
> inconsistent with using that macro and I see it's not used just above
> for the lock warning, which I imagine you copied.

As far as I can tell RelationGetRelationName is for extracting the name
from a Relation struct, but in this case we have a RangeVar so it doesn't appear
to be applicable. I could not find an equivalent access macro for RangeVar.

Thanks again.

Cheers
Mike

Attachment Content-Type Size
v4-0001-Implementation-of-the-ONLY-feature.patch application/octet-stream 15.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message jian he 2024-09-09 08:31:33 Re: not null constraints, again
Previous Message Amit Kapila 2024-09-09 07:42:41 Re: Invalid Assert while validating REPLICA IDENTITY?