Re: Remove unused fields in ReorderBufferTupleBuf

Hi,

> > Here is the corrected patch.
>
> Thank you for updating the patch! I have some comments:

Thanks for the review.

> -        tuple = (ReorderBufferTupleBuf *)
> +        tuple = (HeapTuple)
>                  MemoryContextAlloc(rb->tup_context,
> -
> sizeof(ReorderBufferTupleBuf) +
> +                                                   HEAPTUPLESIZE +
>                                                     MAXIMUM_ALIGNOF +
> alloc_len);
> -        tuple->alloc_tuple_size = alloc_len;
> -        tuple->tuple.t_data = ReorderBufferTupleBufData(tuple);
> +        tuple->t_data = (HeapTupleHeader)((char *)tuple + HEAPTUPLESIZE);
>
> Why do we need to add MAXIMUM_ALIGNOF? since HEAPTUPLESIZE is the
> MAXALIGN'd size, it seems we don't need it. heap_form_tuple() does a
> similar thing but it doesn't add it.

Indeed. I gave it a try and nothing crashed, so it appears that
MAXIMUM_ALIGNOF is not needed.

> ---
>                                  xl_parameter_change *xlrec =
> -                                        (xl_parameter_change *)
> XLogRecGetData(buf->record);
> +                                (xl_parameter_change *)
> XLogRecGetData(buf->record);
>
> Unnecessary change.

That's pgindent. Fixed.

> ---
> -/*
> - * Free a ReorderBufferTupleBuf.
> - */
> -void
> -ReorderBufferReturnTupleBuf(ReorderBuffer *rb, ReorderBufferTupleBuf *tuple)
> -{
> -        pfree(tuple);
> -}
> -
>
> Why does ReorderBufferReturnTupleBuf need to be moved from
> reorderbuffer.c to reorderbuffer.h? It seems not related to this
> refactoring patch so I think we should do it in a separate patch if we
> really want it. I'm not sure it's necessary, though.

OK, fixed.

-- 
Best regards,
Aleksander Alekseev