Hi, Thanks for looking!
On 2019-03-05 18:27:45 -0800, Ashwin Agrawal wrote: > While playing with the tableam, usage of which starts with commit > v12-0023-tableam-Introduce-and-use-begin-endscan-and-do-i.patch, should we > check for NULL function pointer before actually calling the same and ERROR > out instead as NOT_SUPPORTED or something on those lines. Scans seem like absolutely required part of the functionality, so I don't think there's much point in that. It'd just bloat code and runtime. > Understand its kind of think which should get caught during development. > But still currently it segfaults if missing to define some AM function, The segfault iself doesn't bother me at all, it's just a NULL pointer dereference. If we were to put Asserts somewhere it'd crash very similarly. I think you have a point in that: > might be easier for iterative development to error instead in common place. Would make it a tiny bit easier to implement a new AM. We could probably add a few asserts to GetTableAmRoutine(), to check that required functions are implemted. Don't think that'd make a meaningful difference for something like the scan functions, but it'd probably make it easier to forward port AMs to the next release - I'm pretty sure we're going to add required callbacks in the next few releases. Greetings, Andres Freund