Hi, On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote: > index_update_stats() calls RelationGetNumberOfBlocks(<table>). If the AM > doesn't use normal data files, that won't work. I bumped into that with my > toy implementation, which wouldn't need to create any data files, if it > wasn't for this.
There are a few more of these: 1) index_update_stats(), computing pg_class.relpages Feels like the number of both heap and index blocks should be computed by the index build and stored in IndexInfo. That'd also get a bit closer towards allowing indexams not going through smgr (useful e.g. for memory only ones). 2) commands/analyze.c, computing pg_class.relpages This should imo be moved to the tableam callback. It's currently done a bit weirdly imo, with fdws computing relpages the callback, but then also returning the acquirefunc. Seems like it should entirely be computed as part of calling acquirefunc. 3) nodeTidscan, skipping over too large tids I think this should just be moved into the AMs, there's no need to have this in nodeTidscan.c 4) freespace.c, used for the new small-rels-have-no-fsm paths. That's being revised currently anyway. But I'm not particularly concerned even if it stays as is - freespace use is optional anyway. And I can't quite see an AM that doesn't want to use postgres' storage mechanism wanting to use freespace.c Therefore I'm inclined not to thouch this independent of fixing the others. I think none of these are critical issues for tableam, but we should fix them. I'm not sure about doing so for v12 though. 1) and 3) are fairly trivial, but 2) would involve changing the FDW interface, by changing the AnalyzeForeignTable, AcquireSampleRowsFunc signatures. But OTOH, we're not even in beta1. Comments? Greetings, Andres Freund
