Thread: Update obsolete text in indexam.sgml

ISTM it would be better to update the text about index cost estimation in
indexam.sgml.  Please find attached a patch.

Thanks,

Best regards,
Etsuro Fujita

"Etsuro Fujita" <fujita.etsuro@lab.ntt.co.jp> writes:
> ISTM it would be better to update the text about index cost estimation in
> indexam.sgml.  Please find attached a patch.

I'm not too thrilled with the proposed patch.  In the first place, I
don't think it's necessary to address costing of index order-by
expressions in an introductory explanation.  It seems likely that no FDW
will ever need to deal with that at all.  In the second, this change
makes the code less clear, not more so, because it introduces a variable
indexQuals without showing where you would get that value from.
        regards, tom lane

> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]

> "Etsuro Fujita" <fujita.etsuro@lab.ntt.co.jp> writes:
> > ISTM it would be better to update the text about index cost estimation in
> > indexam.sgml.  Please find attached a patch.
>
> I'm not too thrilled with the proposed patch.  In the first place, I
> don't think it's necessary to address costing of index order-by
> expressions in an introductory explanation.

Agreed.

> In the second, this change
> makes the code less clear, not more so, because it introduces a variable
> indexQuals without showing where you would get that value from.

Agreed.  However, I am concerned about the next comment in the current code:

/** Our generic assumption is that the index pages will be read* sequentially, so they cost seq_page_cost each, not
random_page_cost.*...
 

I think this assumption is completely wrong, which has given me a motivation to
propose a patch, though I am missing something.

Thanks,

Best regards,
Etsuro Fujita

"Etsuro Fujita" <fujita.etsuro@lab.ntt.co.jp> writes:
> Agreed.  However, I am concerned about the next comment in the current code:

> /*
>  * Our generic assumption is that the index pages will be read
>  * sequentially, so they cost seq_page_cost each, not random_page_cost.
>  * ...

> I think this assumption is completely wrong, which has given me a motivation to
> propose a patch, though I am missing something.

Mph.  It's pretty hard to argue that it's wrong without considering a
specific index implementation, which in practice would have a ton of
other details that need to be accounted for here.  I don't have a strong
objection to changing the sample code to use random_page_cost instead,
but I doubt it will help anybody one way or another.

FWIW, the docs' sample code was an accurate transcription of what
genericcostestimate did at the time (8.1 era).  I think the case we were
thinking of when that code was written was a recently-rebuilt btree
index, in which logically adjacent leaf pages would indeed often be
sequential.
        regards, tom lane

> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]

> "Etsuro Fujita" <fujita.etsuro@lab.ntt.co.jp> writes:
> > Agreed.  However, I am concerned about the next comment in the current code:
>
> > /*
> >  * Our generic assumption is that the index pages will be read
> >  * sequentially, so they cost seq_page_cost each, not random_page_cost.
> >  * ...
>
> > I think this assumption is completely wrong, which has given me a motivation
> to
> > propose a patch, though I am missing something.
>
> Mph.  It's pretty hard to argue that it's wrong without considering a
> specific index implementation, which in practice would have a ton of
> other details that need to be accounted for here.  I don't have a strong
> objection to changing the sample code to use random_page_cost instead,
> but I doubt it will help anybody one way or another.

To avoid creating unnecessary confusion among Index AM developers, I think it
would be better that the docs' sample code is consistent with the actual code in
selfuncs.c, which the docs referred to at the end of the page.  No?

Thanks,

Best regards,
Etsuro Fujita