Re: extensible external toast tuple support

On 2013-06-18 10:13:39 -0700, Hitoshi Harada wrote:
> On Tue, Jun 18, 2013 at 1:58 AM, Andres Freund <andres(at)2ndquadrant(dot)com>wrote:
>
> > On 2013-06-18 00:56:17 -0700, Hitoshi Harada wrote:
> > > On Fri, Jun 14, 2013 at 4:06 PM, Andres Freund <andres(at)2ndquadrant(dot)com
> > >wrote:
> > >
> > > >
> > > > Here's the updated version. It shouldn't contain any obvious WIP pieces
> > > > anymore, although I think it needs some more documentation. I am just
> > > > not sure where to add it yet, postgres.h seems like a bad place :/
> > > >
> > > >
> > > I have a few comments and questions after reviewing this patch.
> >
> > Cool!
> >
> > > - heap_tuple_fetch_attr doesn't need to be updated to reflect ONDISK
> > macro?
> >
> > It calls toast_fetch_datum() for any kind of external datum, so it
> > should be fine since ONDISK is handled in there.
> >
> >
> toast_fetch_datum doesn't expect the input is INDIRECT. At least I see the
> code path in the same file around toast_insert_or_update() where we have a
> chance to (possibly accidentally) try to fetch ONDISK toasted value from
> non-ONDISK datum.

Hm. Yes. I don't think that's really possible in any codepath I have
thought of - or tested - but that's not a good reason not to make it robust.

> > - -1 from me to use enum for tag types, as I don't think it needs to be.
> > > This looks more like other "kind" macros such as relkind, but I know
> > > there's pros/cons
> >
> > Well, relkind cannot easily be a enum because it needs to be stored in a
> > char field. I like using enums because it gives you the chance of using
> > switch()es at some point and getting warned about missed cases.
> >
> > Why do you dislike it?
> >
> >
> > I put -1 just because it doesn't have to be now. If you argue relkind is
> char field, tag is also uint8.

Well, but to expose it to sql you need a numeric value that actually
translates to a visible ascii character.

> Maybe you can create a user-defined function that creates such datum for
> testing, just in order to demonstrate it works fine.

Hm. Will try whether I can think of something not completely pointless
;)

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services