On Thu, Jan 7, 2016 at 7:41 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> You're stealthily introducing a new abstraction called "string",
> including a typedef and DatumGetString support macros. Is that really
> necessary? Shouldn't it be discussed specifically? I don't necessarily
> oppose it as is, mainly because it's limited to within varlena.c for
> now, but I'm not sure it'd okay to make it more public.
Note that a similar abstraction for the "unknown" type also exists
only within varlena.c. So, DatumGetStringP() and so on appear right
alongside DatumGetUnknownP() and so on.
The idea of the "string" abstraction is that is advertises that
certain functions could equally well apply to a variety of "varlena
header + some bytes" types. I thought about just using the varlena
type instead, but preferred the "string" abstraction.
> Also, there's a lot of churn in this patch to just remove tss to sss.
> Can't we just keep the original variable name?
I think that minimizing lines changed in a mechanical way by a commit
is overrated as a goal for Postgres patches, but I don't feel too
strongly about holding on to the "churn" in this patch. I attach a new
revision, which has the changes I outlined to code comments. I haven't
minimized the differences in the way you suggest just yet.
--
Peter Geoghegan
