Bharath Rupireddy writes:
> How about making the below else if statement and the attname
> assignment into a single line? They are falling below the 80 char
> limit.
> else if (colno > 0 && colno <= list_length(rte->eref->colnames))
> attname = strVal(list_nth(rte->eref->colnam
On Sat, Jul 3, 2021 at 10:03 PM Tom Lane wrote:
>
> Bharath Rupireddy writes:
> > The patch basically looks good to me except a minor comment to have a
> > local variable for var->varattno which makes the code shorter.
>
> Here's a restructured version that uses rangetable data for the
> simple-r
Bharath Rupireddy writes:
> The patch basically looks good to me except a minor comment to have a
> local variable for var->varattno which makes the code shorter.
Here's a restructured version that uses rangetable data for the
simple-relation case too. I also modified the relevant test cases
so
Bharath Rupireddy writes:
> Isn't it better to have the below comment before
> slot_store_error_callback, similar to what's there before
> conversion_error_callback in v7-0002.
> * Note that this function mustn't do any catalog lookups, since we are in
> * an already-failed transaction.
Not rea
On Sat, Jul 3, 2021 at 2:19 AM Tom Lane wrote:
>
> I wrote:
> > Didn't look at 0002 yet.
>
> ... and now that I have, I don't like it much. It adds a lot of
> complexity, plus some lookup cycles that might be wasted. I experimented
> with looking into the range table as I suggested previously, a
On Sat, Jul 3, 2021 at 1:37 AM Tom Lane wrote:
>
> Bharath Rupireddy writes:
> >> << Attaching v5-0001 here again for completion >>
> >> I'm attaching v5-0001 patch that avoids printing the column type names
> >> in the context message thus no cache lookups have to be done in the
> >> error conte
I wrote:
> Didn't look at 0002 yet.
... and now that I have, I don't like it much. It adds a lot of
complexity, plus some lookup cycles that might be wasted. I experimented
with looking into the range table as I suggested previously, and
that seems to work; see attached. (This includes a little
Bharath Rupireddy writes:
>> << Attaching v5-0001 here again for completion >>
>> I'm attaching v5-0001 patch that avoids printing the column type names
>> in the context message thus no cache lookups have to be done in the
>> error context callback. I think the column name is enough to know on
>>
On Fri, Apr 16, 2021 at 3:23 PM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
>
> On Tue, Mar 16, 2021 at 2:21 AM Tom Lane wrote:
> >
> > Bharath Rupireddy writes:
> > > Thanks for pointing to the changes in the commit message. I corrected
> > > them. Attaching v4 patch set,
On Tue, Mar 16, 2021 at 2:21 AM Tom Lane wrote:
>
> Bharath Rupireddy writes:
> > Thanks for pointing to the changes in the commit message. I corrected
> > them. Attaching v4 patch set, consider it for further review.
>
> I took a quick look at this. I'm quite worried about the potential
> perfo
On Wed, Mar 17, 2021 at 4:52 PM Bharath Rupireddy
wrote:
>
> On Tue, Mar 16, 2021 at 2:21 AM Tom Lane wrote:
> > > Thanks for pointing to the changes in the commit message. I corrected
> > > them. Attaching v4 patch set, consider it for further review.
> >
> > I took a quick look at this. I'm qu
On Tue, Mar 16, 2021 at 2:21 AM Tom Lane wrote:
> > Thanks for pointing to the changes in the commit message. I corrected
> > them. Attaching v4 patch set, consider it for further review.
>
> I took a quick look at this. I'm quite worried about the potential
> performance cost of the v4-0001 patc
Bharath Rupireddy writes:
> Thanks for pointing to the changes in the commit message. I corrected
> them. Attaching v4 patch set, consider it for further review.
I took a quick look at this. I'm quite worried about the potential
performance cost of the v4-0001 patch (the one for fixing
slot_stor
On Thu, Feb 4, 2021 at 5:42 PM Amit Kapila wrote:
> > Thanks Amit. I verified it with gdb. I attached gdb to the logical
> > replication worker. In slot_store_data's for loop, I intentionally set
> > CurrentTransactionState->state = TRANS_DEFAULT,
> >
>
> What happens if you don't change CurrentTr
Hi,
On 2021-02-04 15:08:42 +0530, Bharath Rupireddy wrote:
> On Thu, Feb 4, 2021 at 5:16 AM Andres Freund wrote:
> > > > > About 0001, have we tried to reproduce the actual bug here which means
> > > > > when the error_callback is called we should face some problem? I feel
> > > > > with the corr
On Thu, Feb 4, 2021 at 5:38 PM Bharath Rupireddy
wrote:
>
> On Thu, Feb 4, 2021 at 4:00 PM Amit Kapila wrote:
> > > > About 0001, have we tried to reproduce the actual bug here which means
> > > > when the error_callback is called we should face some problem? I feel
> > > > with the correct testc
On Thu, Feb 4, 2021 at 4:00 PM Amit Kapila wrote:
> > > About 0001, have we tried to reproduce the actual bug here which means
> > > when the error_callback is called we should face some problem? I feel
> > > with the correct testcase we should hit the Assert
> > > (Assert(IsTransactionState());)
On Wed, Feb 3, 2021 at 4:31 PM Bharath Rupireddy
wrote:
>
> On Thu, Jan 28, 2021 at 11:14 AM Amit Kapila wrote:
> >
> > On Wed, Jan 27, 2021 at 9:38 AM Bharath Rupireddy
> > wrote:
> > >
> > > On Wed, Jan 27, 2021 at 7:48 AM Zhihong Yu wrote:
> > >
> > > Thanks for pointing to the changes in th
On Thu, Feb 4, 2021 at 5:16 AM Andres Freund wrote:
> > > > About 0001, have we tried to reproduce the actual bug here which means
> > > > when the error_callback is called we should face some problem? I feel
> > > > with the correct testcase we should hit the Assert
> > > > (Assert(IsTransactionS
Hi,
On 2021-02-03 16:35:29 +0530, Bharath Rupireddy wrote:
> On Sat, Jan 30, 2021 at 8:23 AM Andres Freund wrote:
> > On 2021-01-28 11:14:03 +0530, Amit Kapila wrote:
> > > On Wed, Jan 27, 2021 at 9:38 AM Bharath Rupireddy
> > > wrote:
> > > >
> > > > On Wed, Jan 27, 2021 at 7:48 AM Zhihong Yu
On Sat, Jan 30, 2021 at 8:23 AM Andres Freund wrote:
>
> Hi,
>
> On 2021-01-28 11:14:03 +0530, Amit Kapila wrote:
> > On Wed, Jan 27, 2021 at 9:38 AM Bharath Rupireddy
> > wrote:
> > >
> > > On Wed, Jan 27, 2021 at 7:48 AM Zhihong Yu wrote:
> > >
> > > Thanks for pointing to the changes in the c
On Thu, Jan 28, 2021 at 11:14 AM Amit Kapila wrote:
>
> On Wed, Jan 27, 2021 at 9:38 AM Bharath Rupireddy
> wrote:
> >
> > On Wed, Jan 27, 2021 at 7:48 AM Zhihong Yu wrote:
> >
> > Thanks for pointing to the changes in the commit message. I corrected
> > them. Attaching v4 patch set, consider it
Hi,
On 2021-01-28 11:14:03 +0530, Amit Kapila wrote:
> On Wed, Jan 27, 2021 at 9:38 AM Bharath Rupireddy
> wrote:
> >
> > On Wed, Jan 27, 2021 at 7:48 AM Zhihong Yu wrote:
> >
> > Thanks for pointing to the changes in the commit message. I corrected
> > them. Attaching v4 patch set, consider it
On Wed, Jan 27, 2021 at 9:38 AM Bharath Rupireddy
wrote:
>
> On Wed, Jan 27, 2021 at 7:48 AM Zhihong Yu wrote:
>
> Thanks for pointing to the changes in the commit message. I corrected
> them. Attaching v4 patch set, consider it for further review.
>
About 0001, have we tried to reproduce the ac
On Wed, Jan 27, 2021 at 7:48 AM Zhihong Yu wrote:
> For 0002, a few comments on the description:
>
> bq. Avoid accessing system catalogues inside conversion_error_callback
>
> catalogues -> catalog
>
> bq. error context callback, because the the entire transaction might
>
> There is redundant 'the
For 0002, a few comments on the description:
bq. Avoid accessing system catalogues inside conversion_error_callback
catalogues -> catalog
bq. error context callback, because the the entire transaction might
There is redundant 'the'
bq. Since get_attname() and get_rel_name() could search the sy
On Tue, Jan 26, 2021 at 11:29 AM Masahiko Sawada wrote:
> > IMHO, adding an assertion in SearchCatCacheInternal(which is a most
> > generic code part within the server) with a few error context global
> > variables may not be always safe. Because we might miss using the
> > error context variables
On Tue, Jan 12, 2021 at 6:33 PM Bharath Rupireddy
wrote:
>
> On Tue, Jan 12, 2021 at 9:40 AM Masahiko Sawada wrote:
> >
> > On Mon, Jan 11, 2021 at 4:46 PM Bharath Rupireddy
> > wrote:
> > >
> > > On Mon, Jan 11, 2021 at 10:56 AM Masahiko Sawada
> > > wrote:
> > > > Agreed. Attached the update
On Tue, Jan 12, 2021 at 9:40 AM Masahiko Sawada wrote:
>
> On Mon, Jan 11, 2021 at 4:46 PM Bharath Rupireddy
> wrote:
> >
> > On Mon, Jan 11, 2021 at 10:56 AM Masahiko Sawada
> > wrote:
> > > Agreed. Attached the updated patch.
> >
> > Thanks for the updated patch. Looks like the comment crosse
On Mon, Jan 11, 2021 at 4:46 PM Bharath Rupireddy
wrote:
>
> On Mon, Jan 11, 2021 at 10:56 AM Masahiko Sawada
> wrote:
> > Agreed. Attached the updated patch.
>
> Thanks for the updated patch. Looks like the comment crosses the 80
> char limit, I have corrected it. And also changed the variable
On Mon, Jan 11, 2021 at 10:56 AM Masahiko Sawada wrote:
> Agreed. Attached the updated patch.
Thanks for the updated patch. Looks like the comment crosses the 80
char limit, I have corrected it. And also changed the variable name
from remotetypeid to remotetypid, so that logicalrep_typmap_gettypn
On Sat, Jan 9, 2021 at 2:57 PM Bharath Rupireddy
wrote:
>
> On Thu, Jan 7, 2021 at 5:47 AM Masahiko Sawada wrote:
> >
> > On Wed, Jan 6, 2021 at 11:42 AM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, Jan 6, 2021 at 11:02 AM Andres Freund wrote:
> > > >
> > > > Hi,
> > > >
> > > > Due to a deb
On Thu, Jan 7, 2021 at 5:47 AM Masahiko Sawada wrote:
>
> On Wed, Jan 6, 2021 at 11:42 AM Masahiko Sawada wrote:
> >
> > On Wed, Jan 6, 2021 at 11:02 AM Andres Freund wrote:
> > >
> > > Hi,
> > >
> > > Due to a debug ereport I just noticed that worker.c's
> > > slot_store_error_callback is doing
On Wed, Jan 6, 2021 at 11:42 AM Masahiko Sawada wrote:
>
> On Wed, Jan 6, 2021 at 11:02 AM Andres Freund wrote:
> >
> > Hi,
> >
> > Due to a debug ereport I just noticed that worker.c's
> > slot_store_error_callback is doing something quite dangerous:
> >
> > static void
> > slot_store_error_call
On Wed, Jan 6, 2021 at 11:02 AM Andres Freund wrote:
>
> Hi,
>
> Due to a debug ereport I just noticed that worker.c's
> slot_store_error_callback is doing something quite dangerous:
>
> static void
> slot_store_error_callback(void *arg)
> {
> SlotErrCallbackArg *errarg = (SlotErrCallbackA
Hi,
Due to a debug ereport I just noticed that worker.c's
slot_store_error_callback is doing something quite dangerous:
static void
slot_store_error_callback(void *arg)
{
SlotErrCallbackArg *errarg = (SlotErrCallbackArg *) arg;
LogicalRepRelMapEntry *rel;
char *remot
36 matches
Mail list logo
