|From:||Andres Freund <andres(at)anarazel(dot)de>|
|To:||Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>|
|Cc:||Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org|
|Subject:||Re: fast defaults in heap_getattr vs heap_deform_tuple|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 2019-02-05 22:44:38 -0300, Alvaro Herrera wrote:
> On 2019-Feb-05, Andres Freund wrote:
> > @@ -82,7 +80,7 @@ static Datum getmissingattr(TupleDesc tupleDesc, int attnum, bool *isnull);
> > /*
> > * Return the missing value of an attribute, or NULL if there isn't one.
> > */
> > -static Datum
> > +Datum
> > getmissingattr(TupleDesc tupleDesc,
> > int attnum, bool *isnull)
> This is a terrible name for an exported function -- let's change it
> before it gets exported. Heck, even heap_getmissingattr() would be
I don't really aggree. Note that the relevant datastructure is named
AttrMissing, and that the function isn't relevant for heap tuple. So I
don't really see what we'd otherwise name it? I think if we wanted to
improve this we should start with AttrMissing and not this function.
> I notice that with this patch, heap_getattr() obtains a new Assert()
> that the attr being fetched is no further than tupledesc->natts.
> It previously just returned null for that case. Maybe we should change
> it so that it returns null if an attr beyond end-of-array is fetched?
> (I think in non-assert builds, it would dereference past the AttrMissing
Hm, it seems like there's plenty issues with accessing datums after the
end of the desc before this change, as well as after this change. Note
that slot_getattr() (in 11, it looks a bit different in master) already
calls getmissingattr(). And that's much more commonly used. Therefore
I'm disinclined to see this as a problem?
|Next Message||Andres Freund||2019-02-09 10:54:00||Re: fast defaults in heap_getattr vs heap_deform_tuple|
|Previous Message||Robert Haas||2019-02-09 10:21:12||Re: dsa_allocate() faliure|