At 2015-05-09 02:20:51 +0200, and...@anarazel.de wrote:
>
> > + * Abhijit Menon-Sen <a...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to