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