Re: fast defaults in heap_getattr vs heap_deform_tuple
От | Andres Freund |
---|---|
Тема | Re: fast defaults in heap_getattr vs heap_deform_tuple |
Дата | |
Msg-id | 20190209105155.465bq5qvijj2smkg@alap3.anarazel.de обсуждение исходный текст |
Ответ на | Re: fast defaults in heap_getattr vs heap_deform_tuple (Alvaro Herrera <alvherre@2ndquadrant.com>) |
Список | pgsql-hackers |
Hi, 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 > better. 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 > array.) 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? Greetings, Andres Freund
В списке pgsql-hackers по дате отправления: