Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ndl7r-0007na-K6 for pgsql-hackers@arkaria.postgresql.org; Mon, 11 Apr 2022 03:54:03 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1ndl7q-0005nf-2F for pgsql-hackers@arkaria.postgresql.org; Mon, 11 Apr 2022 03:54:02 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ndl7p-0005nV-Mq for pgsql-hackers@lists.postgresql.org; Mon, 11 Apr 2022 03:54:01 +0000 Received: from mail-lj1-x230.google.com ([2a00:1450:4864:20::230]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1ndl7m-0004jK-UE for pgsql-hackers@postgresql.org; Mon, 11 Apr 2022 03:54:01 +0000 Received: by mail-lj1-x230.google.com with SMTP id r18so2869874ljp.0 for ; Sun, 10 Apr 2022 20:53:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yugabyte.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jWQ5wfnC7tKp7+9i0edmJQNXeAj/JUuyojyc7DNcU3Y=; b=U2w9XdbP/4M5aEiYtJqP/DDHNmOkISUvfX6MyJwT67bw28CLVYu5wjZhE1UjzIEzeW OxnWVgKuG2GaAJ2T7fxX+nCOK8CfnRhpew+cJaEJOeH5Y6F3TsSe+9Ss7PWaEZcpVf8E c/2jq866jrzqIAV68miCDvaZCQAPdHkhspHRU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=jWQ5wfnC7tKp7+9i0edmJQNXeAj/JUuyojyc7DNcU3Y=; b=u8JrP63qeHY7UsgE1s+Ld6TtKq93E/UzpLx04BgQvQKux6fMHU3QGZIFkQaU+of8UC pgRggvJ7/51+P/rnvMGjl7I29WQ3rlifuFdQOh1vbbzazv4jPGukvT3V8Bv6TSLY2l8q KdpqwCkgDQVZN/CUtEs5xGyaIWcOKFKJplQ7Iwtx890zEQPXYzOorbm2LK8zicNEoQ3i T+XdoBbroAqAvhLbTv5FRmUhqLFBepLlpQ4+F5bE7CUdieixMfx8w3niki3nsMq4rvLk m1KHKSLT6QP/9LrsTKimR4+wS6/u8CtloeUbUNBYnfHmFM3pSgxTKyTBt3HBc7AzwXQG LNbw== X-Gm-Message-State: AOAM531oYCwf+EQGxim1ro1KsObU/zGLzUY2krGnZBR7M9JI+QHc7SLs 85mIrxKw3Zx3CI/1pL4dHezBtr4k54g9Ew5xoGLEMw== X-Google-Smtp-Source: ABdhPJxuB1TjfjCQ2rdYP0xl2/4NIp22TWv2gZRh/P/Y8DRyWC0cR0fxNH5lj9xFA/bUAJT/iOxe/7N/1ZFDYy4N+no= X-Received: by 2002:a2e:570d:0:b0:24b:5058:bc8d with SMTP id l13-20020a2e570d000000b0024b5058bc8dmr9017136ljb.263.1649649236517; Sun, 10 Apr 2022 20:53:56 -0700 (PDT) MIME-Version: 1.0 References: <215356.1647286703@sss.pgh.pa.us> In-Reply-To: From: Zhihong Yu Date: Sun, 10 Apr 2022 20:58:23 -0700 Message-ID: Subject: Re: generic plans and "initial" pruning To: Amit Langote Cc: David Rowley , Robert Haas , Tom Lane , PostgreSQL-development Content-Type: multipart/alternative; boundary="000000000000bf3afe05dc58e47f" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000bf3afe05dc58e47f Content-Type: text/plain; charset="UTF-8" On Sun, Apr 10, 2022 at 8:05 PM Amit Langote wrote: > On Fri, Apr 8, 2022 at 8:45 PM Amit Langote > wrote: > > Most looked fine changes to me except a couple of typos, so I've > > adopted those into the attached new version, even though I know it's > > too late to try to apply it. > > > > + * XXX is it worth doing a bms_copy() on glob->minLockRelids if > > + * glob->containsInitialPruning is true?. I'm slighly worried that the > > + * Bitmapset could have a very long empty tail resulting in excessive > > + * looping during AcquireExecutorLocks(). > > + */ > > > > I guess I trust your instincts about bitmapset operation efficiency > > and what you've written here makes sense. It's typical for leaf > > partitions to have been appended toward the tail end of rtable and I'd > > imagine their indexes would be in the tail words of minLockRelids. If > > copying the bitmapset removes those useless words, I don't see why we > > shouldn't do that. So added: > > > > + /* > > + * It seems worth doing a bms_copy() on glob->minLockRelids if we > deleted > > + * bit from it just above to prevent empty tail bits resulting in > > + * inefficient looping during AcquireExecutorLocks(). > > + */ > > + if (glob->containsInitialPruning) > > + glob->minLockRelids = bms_copy(glob->minLockRelids) > > > > Not 100% about the comment I wrote. > > And the quoted code change missed a semicolon in the v14 that I > hurriedly sent on Friday. (Had apparently forgotten to `git add` the > hunk to fix that). > > Sending v15 that fixes that to keep the cfbot green for now. > > -- > Amit Langote > EDB: http://www.enterprisedb.com Hi, + /* RT index of the partitione table. */ partitione -> partitioned Cheers --000000000000bf3afe05dc58e47f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Sun, Apr 10, 2022 at 8:05 PM Amit = Langote <amitlangote09@gmail.= com> wrote:
On Fri, Apr 8, 2022 at 8:45 PM Amit Langote <amitlangote09@gmail.com> wrote:=
> Most looked fine changes to me except a couple of typos, so I've > adopted those into the attached new version, even though I know it'= ;s
> too late to try to apply it.
>
> + * XXX is it worth doing a bms_copy() on glob->minLockRelids if > + * glob->containsInitialPruning is true?. I'm slighly worried = that the
> + * Bitmapset could have a very long empty tail resulting in excessive=
> + * looping during AcquireExecutorLocks().
> + */
>
> I guess I trust your instincts about bitmapset operation efficiency > and what you've written here makes sense.=C2=A0 It's typical f= or leaf
> partitions to have been appended toward the tail end of rtable and I&#= 39;d
> imagine their indexes would be in the tail words of minLockRelids.=C2= =A0 If
> copying the bitmapset removes those useless words, I don't see why= we
> shouldn't do that.=C2=A0 So added:
>
> + /*
> + * It seems worth doing a bms_copy() on glob->minLockRelids if we = deleted
> + * bit from it just above to prevent empty tail bits resulting in
> + * inefficient looping during AcquireExecutorLocks().
> + */
> + if (glob->containsInitialPruning)
> + glob->minLockRelids =3D bms_copy(glob->minLockRelids)
>
> Not 100% about the comment I wrote.

And the quoted code change missed a semicolon in the v14 that I
hurriedly sent on Friday.=C2=A0 =C2=A0(Had apparently forgotten to `git add= ` the
hunk to fix that).

Sending v15 that fixes that to keep the cfbot green for now.

--
Amit Langote
EDB: http://www.enterprisedb.com
Hi,

<= /div>
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* RT index of= the partitione table. */

partitione ->=C2=A0pa= rtitioned

Cheers
--000000000000bf3afe05dc58e47f--