Thread: bug in ReplicationSlotsCountDBSlots

bug in ReplicationSlotsCountDBSlots

From
yamamoto@valinux.co.jp (YAMAMOTO Takashi)
Date:
hi,

the following fixes an obvious bug in ReplicationSlotsCountDBSlots
which prevents dropping unrelated databases.

YAMAMOTO Takashi

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 45ed7e4..76e5573 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -727,6 +727,7 @@ ReplicationSlotsCountDBSlots(Oid dboid, int *nslots, int *nactive)

         /* not database specific, skip */
         if (s->data.database == InvalidOid)
+            continue;

         /* not our database, skip */
         if (s->data.database != dboid)

Re: bug in ReplicationSlotsCountDBSlots

From
Bruce Momjian
Date:
On Fri, Mar  7, 2014 at 11:43:21AM +0900, YAMAMOTO Takashi wrote:
> hi,
>
> the following fixes an obvious bug in ReplicationSlotsCountDBSlots
> which prevents dropping unrelated databases.

Yikes, the code is literally:

        /* not database specific, skip */
        if (s->data.database == InvalidOid)

How did that get by the reviewers?

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

Re: bug in ReplicationSlotsCountDBSlots

From
Andres Freund
Date:
On 2014-03-07 11:18:04 -0500, Bruce Momjian wrote:
> On Fri, Mar  7, 2014 at 11:43:21AM +0900, YAMAMOTO Takashi wrote:
> > hi,
> >
> > the following fixes an obvious bug in ReplicationSlotsCountDBSlots
> > which prevents dropping unrelated databases.

Thanks, that's obviously a mistake!

> Yikes, the code is literally:
>
>         /* not database specific, skip */
>         if (s->data.database == InvalidOid)
>
> How did that get by the reviewers?

It's a huge patch, and it changed a lot to accomodate review and to
rebase onto new versions. I think I introduced that bug when rebasing
the decoding functionality onto the already committed slot
functionality. Since it doesn't cause any obvious problems in many
scenarios..

Can you push that fix?

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: bug in ReplicationSlotsCountDBSlots

From
Bruce Momjian
Date:
On Fri, Mar  7, 2014 at 05:22:03PM +0100, Andres Freund wrote:
> On 2014-03-07 11:18:04 -0500, Bruce Momjian wrote:
> > On Fri, Mar  7, 2014 at 11:43:21AM +0900, YAMAMOTO Takashi wrote:
> > > hi,
> > >
> > > the following fixes an obvious bug in ReplicationSlotsCountDBSlots
> > > which prevents dropping unrelated databases.
>
> Thanks, that's obviously a mistake!
>
> > Yikes, the code is literally:
> >
> >         /* not database specific, skip */
> >         if (s->data.database == InvalidOid)
> >
> > How did that get by the reviewers?
>
> It's a huge patch, and it changed a lot to accomodate review and to
> rebase onto new versions. I think I introduced that bug when rebasing
> the decoding functionality onto the already committed slot
> functionality. Since it doesn't cause any obvious problems in many
> scenarios..
>
> Can you push that fix?

Done, thanks.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +