Re: Review: Patch to compute Max LSN of Data Pages

Hi Amit

On Mon, Dec 3, 2012 at 6:56 PM, Amit kapila <amit(dot)kapila(at)huawei(dot)com> wrote:

> >I think we should expect provided path to be relative to current
> directory
> > or may consider it to be relative to either one of Data or CWD.
> >Because normally we expect path to be relative to CWD if some program is
> > asking for path in command line.
>
> Please find the attached patch to make the path relative to CWD and check
> if the path is under data directory.
>

Works good now. Although I am thinking why are you disallowing the absolute
path of file. Any particular reason?

>
> Now the only point raised by Alvaro and you for this patch which is not
> yet addressed is :
>
> > Hmm. I think I'd expect that if I give pg_computemaxlsn a number, it
> > should consider that it is a relfilenode, and so it should get a list of
> > all segments for all forks of it. So if I give "12345" it should get
> > 12345, 12345.1 and so on, and also 12345_vm 12345_vm.1 and so on.
> > However, if what I give it is a path, i.e. it contains a slash, I think
> > it should only consider the specific file mentioned. In that light, I'm
> > not sure that command line options chosen are the best set.
> I am just not sure whether we should handle this functionality and if we
> have to handle what is better way to provide it to user.
> Shall we provide new option -r or something for it.
>
> Opinions/Suggestions?
>
> IMHO, such functionality can be easily extendable in future.
> However I have no problem in implementing such functionality if you are of
> opinion that this is basic and it should go with first version of feature.
>
>

I also had a similar point made by Alvaro to allow all the segments of the
relation for a given relation file name, or add another option do do the
same. But if everybody is fine with leaving it for the future, I do not
have any further concerns with the patch. It is good from my side.

With Regards,
> Amit Kapila.
>
>

Thanks
Muhammad Usama