At 2015-05-09 02:20:51 +0200, andres@anarazel.de wrote:
>
> > + * Abhijit Menon-Sen <ams@2ndQuadrant.com>
> > + * Portions Copyright (c) 2001,2002 Tatsuo Ishii (from pgstattuple)
>
> I think for new extension we don't really add authors and such anymore.
OK, I'll get rid of the boilerplate.
> Hm. I do wonder if this should really be called 'statbloat'. Perhaps
> it'd more appropriately be called pg_estimate_bloat or somesuch?
Since we've moved it into pgstattuple, perhaps pgstattuple_approximate()
or pgstattuple_estimated() would be a better name. I don't really care,
I'll change it to whatever people like.
> Also, is free_space really exact? The fsm isn't byte exact.
You're right, I'll fix that.
> Why go through C strings? I personally think we really shouldn't add
> more callers to BuildTupleFromCStrings, it's such an absurd interface.
I just copied this more or less blindly from pgstattuple. I'll fix it
and submit a separate patch to fix the equivalent code in pgstattuple.
> I think it'd actually be fine to just say that the relation has to be
> a table or matview.
Good point. Agreed.
> I don't believe that rationale is really true these days. I'd much
> rather just rip this out here than copy the rather complex logic.
Yes, thanks, I very much agree that this isn't really worth copying.
I'll drop it in my next submission.
> I haven't checked, but I'm not sure that it's safe/meaningful to call
> PageGetHeapFreeSpace() on a new page.
OK, I'll check and fix if necessary.
Thanks for the feedback. I'll submit a revised patch shortly.
-- Abhijit