On Fri, Jan 26, 2024 at 4:04 PM Masahiko Sawada wrote:
>
> On Thu, Jan 25, 2024 at 10:17 PM Aleksander Alekseev
> wrote:
> >
> > Hi,
> >
> > > > Here is the corrected patch.
> > >
> > > Thank you for updating the patch! I have some comments:
> >
> > Thanks for the review.
> >
> > > -tuple
On Fri, 2024-01-26 at 13:51 +0900, Masahiko Sawada wrote:
> On Fri, Jan 26, 2024 at 1:24 PM wrote:
> >
> > On Thu, 2024-01-25 at 16:11 -0500, reid.thomp...@crunchydata.com wrote:
> > >
> > > I walked through v6 and didn't note any issues.
>
> Thank you for reviewing the patch!
>
Happy to.
>
>
On Thu, Jan 25, 2024 at 10:17 PM Aleksander Alekseev
wrote:
>
> Hi,
>
> > > Here is the corrected patch.
> >
> > Thank you for updating the patch! I have some comments:
>
> Thanks for the review.
>
> > -tuple = (ReorderBufferTupleBuf *)
> > +tuple = (HeapTuple)
> >
On Fri, Jan 26, 2024 at 1:24 PM wrote:
>
> On Thu, 2024-01-25 at 16:11 -0500, reid.thomp...@crunchydata.com wrote:
> >
> > I walked through v6 and didn't note any issues.
Thank you for reviewing the patch!
> >
> > I do want to ask, the patch alters ReorderBufferReturnTupleBuf() and
> > drops the
On Thu, 2024-01-25 at 16:11 -0500, reid.thomp...@crunchydata.com wrote:
>
> I walked through v6 and didn't note any issues.
>
> I do want to ask, the patch alters ReorderBufferReturnTupleBuf() and
> drops the unused parameter ReorderBuffer *rb. It seems that
> ReorderBufferFreeSnap(), ReorderBuff
On Thu, 2024-01-25 at 16:16 +0300, Aleksander Alekseev wrote:
> Hi,
>
> > > Here is the corrected patch.
> >
> > Thank you for updating the patch! I have some comments:
>
> Thanks for the review.
>
> > -tuple = (ReorderBufferTupleBuf *)
> > +tuple = (HeapTuple)
> >
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) +
> +
Sorry for being absent for a while.
On Mon, Jan 22, 2024 at 11:19 PM Aleksander Alekseev
wrote:
>
> Hi,
>
> > > But why didn't you pursue your idea of getting rid of the wrapper
> > > structure ReorderBufferTupleBuf which after this patch will have just
> > > one member? I think there could be ha
Hi,
> > But why didn't you pursue your idea of getting rid of the wrapper
> > structure ReorderBufferTupleBuf which after this patch will have just
> > one member? I think there could be hassles in backpatching bug-fixes
> > in some cases but in the longer run it would make the code look clean.
>
Hi,
> > I played a bit more with the patch. There was an idea to make
> > ReorderBufferTupleBufData an opaque structure known only within
> > reorderbyffer.c but it turned out that replication/logical/decode.c
> > accesses it directly so I abandoned that idea for now.
> >
> > > Alternatively we co
On Mon, Jan 22, 2024 at 4:17 PM Aleksander Alekseev
wrote:
>
> Hi,
>
> > Hi, this patch was marked in CF as "Needs Review" [1], but there has
> > been no activity on this thread for 5+ months.
> >
> > Do you wish to keep this open, or can you post something to elicit
> > more interest in reviews f
On Wed, Jul 26, 2023 at 7:22 PM Aleksander Alekseev
wrote:
>
> > Here is the corrected patch. I added it to the nearest CF [1].
>
> I played a bit more with the patch. There was an idea to make
> ReorderBufferTupleBufData an opaque structure known only within
> reorderbyffer.c but it turned out th
Hi,
> Hi, this patch was marked in CF as "Needs Review" [1], but there has
> been no activity on this thread for 5+ months.
>
> Do you wish to keep this open, or can you post something to elicit
> more interest in reviews for the latest patch set? Otherwise, if
> nothing happens then the CF entry
2024-01 Commitfest.
Hi, this patch was marked in CF as "Needs Review" [1], but there has
been no activity on this thread for 5+ months.
Do you wish to keep this open, or can you post something to elicit
more interest in reviews for the latest patch set? Otherwise, if
nothing happens then the CF e
Hi,
> Here is the corrected patch. I added it to the nearest CF [1].
I played a bit more with the patch. There was an idea to make
ReorderBufferTupleBufData an opaque structure known only within
reorderbyffer.c but it turned out that replication/logical/decode.c
accesses it directly so I abandone
Hi,
> I'm afraid it's a bit incomplete:
>
> ```
> ../src/backend/replication/logical/reorderbuffer.c: In function
> ‘ReorderBufferGetTupleBuf’:
> ../src/backend/replication/logical/reorderbuffer.c:565:14: error:
> ‘ReorderBufferTupleBuf’ has no member named ‘alloc_tuple_size’
> 565 | tup
Hi,
Thanks for the patch.
> However, node and alloc_tuple_size are not used at all. It seems an
> oversight in commit a4ccc1cef5a, which introduced the generation
> context and used it in logical decoding. I think we can remove them
> (only on HEAD). I've attached the patch.
I'm afraid it's a bi
Hi,
ReorderBufferTupleBuf is defined as follow:
/* an individual tuple, stored in one chunk of memory */
typedef struct ReorderBufferTupleBuf
{
/* position in preallocated list */
slist_node node;
/* tuple header, the interesting bit for users of logical decoding */
HeapTupleDat
18 matches
Mail list logo
