Re: [PATCH] Do not use StdRdOptions in Access Methods

В письме от среда, 20 ноября 2019 г. 16:44:18 MSK пользователь Michael Paquier
написал:

> > It seems to me that if the plan is to have one option structure for
> > each index AM, which has actually the advantage to reduce the bloat of
> > each relcache entry currently relying on StdRdOptions, then we could
> > have those extra assertion checks in the same patch, because the new
> > macros are introduced.
> I have looked at this patch, and did not like much having the
> calculations of the page free space around, so I have moved that into
> each AM's dedicated header.
Sounds as a good idea. I try not touch such things following the rule "is not
broken do not fix" but this way it is definitely better. Thanks!

> > There is rd_rel->relam. You can for example refer to pgstatindex.c
> > which has AM-related checks to make sure that the correct index AM is
> > being used.
>
> We can do something similar for GIN and BRIN on top of the rest, so
> updated the patch with that.
That is what I've been trying to tell speaking about code consistency. But ok,
this way is also good.

> Nikolay, I would be fine to commit this patch as-is.
Yeah. I've read the patch. I like it, actually I started doing same thing
myself but you were faster. I have opportunity to pay attention to postgres
once a week these days...

I like the patch, and also agree that it should be commited as is.

Though I have a notion to think about.

BRIN_AM_OID and friends are defined in catalog/pg_am_d.h so for core indexes
we can do relation->rd_rel->relam == BRIN_AM_OID check. But for contrib
indexes we can't do such a thing.
Bloom index does not need such check as it uses options only when index is
created. At that point you can not choose wrong relation. But if in future we
will have some contrib index that uses options when it some data is inserted
(as it is done with fillfactor in core indexes) then index author will not be
able to do such relam check. I would not call it a big problem, but it is
something to think about, for sure...

> Thanks for your patience on this stuff.
Thaks for joining this work, and sorry for late replies. Now I quite rarely
have time for postgres :-(

--
Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da
Body-oriented Therapist: https://vk.com/nataraj_rebalancing (Russian)